Bug 192013 – Introducing a ENABLE_SEPARATED_WX_HEAP macro.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

This makes the code a little more readable.  See https://bugs.webkit.org/show_bug.cgi?id=190022#c3.

<rdar://problem/45494310>

[Mark Lam]

Created attachment 355749 [details]
proposed patch.

[Keith Miller]

Comment on attachment 355749 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=355749&action=review

r=me.

> Source/JavaScriptCore/config.h:41
> +#if !defined(ENABLE_SEPARATED_WX_HEAP)

Nit: why not #ifndef?

> Source/JavaScriptCore/config.h:42
> +#if (!ENABLE(FAST_JIT_PERMISSIONS) || !CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)

Nit: Can we make this !(ENABLE(FAST_JIT_PERMISSIONS) && CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)?

I prefer to pull the negations out since I find it easier to read the inside of the parens then negate.

[Mark Lam]

Comment on attachment 355749 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=355749&action=review

Thanks for the review.

>> Source/JavaScriptCore/config.h:41
>> +#if !defined(ENABLE_SEPARATED_WX_HEAP)
> 
> Nit: why not #ifndef?

So that I can do #endif // !defined(ENABLE_SEPARATED_WX_HEAP) below.

>> Source/JavaScriptCore/config.h:42
>> +#if (!ENABLE(FAST_JIT_PERMISSIONS) || !CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)
> 
> Nit: Can we make this !(ENABLE(FAST_JIT_PERMISSIONS) && CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)?
> 
> I prefer to pull the negations out since I find it easier to read the inside of the parens then negate.

I prefer the original.  It reads better as the individual conditions for enabling the SEPARATED_WX_HEAP.  I'm going to keep this as is.

[Mark Lam]

Landed in r238564: <http://trac.webkit.org/r238564>.