[webkit-reviews] review granted: [Bug 170770] [JSC] Clean up heap/MachineStackMarker by introducing USE(MACHINE_CONTEXT) : [Attachment 306909] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 12 09:38:58 PDT 2017


Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 170770: [JSC] Clean up heap/MachineStackMarker by introducing
USE(MACHINE_CONTEXT)
https://bugs.webkit.org/show_bug.cgi?id=170770

Attachment 306909: Patch

https://bugs.webkit.org/attachment.cgi?id=306909&action=review




--- Comment #4 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 306909
  --> https://bugs.webkit.org/attachment.cgi?id=306909
Patch

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

r=me with suggestion.

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:235
> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)

Here, you explicitly  call out OS(DARWIN) even though USE(MACHINE_CONTEXT)
already include OS(DARWIN).  Later below, you don't call out OS(DARWIN).  Let's
be consistent.	How about we don't call out OS(DARWIN) explicitly?

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:244
> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)

Ditto.

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:254
> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)

Ditto.


More information about the webkit-reviews mailing list