RESOLVED FIXED 170770
[JSC] Clean up heap/MachineStackMarker by introducing HAVE(MACHINE_CONTEXT)
https://bugs.webkit.org/show_bug.cgi?id=170770
Summary [JSC] Clean up heap/MachineStackMarker by introducing HAVE(MACHINE_CONTEXT)
Yusuke Suzuki
Reported 2017-04-12 07:34:32 PDT
[JSC] Clean up heap/MachineStackMarker by introducing USE(MACHINE_CONTEXT)
Attachments
Patch (15.69 KB, patch)
2017-04-12 07:36 PDT, Yusuke Suzuki
mark.lam: review+
buildbot: commit-queue-
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.05 MB, application/zip)
2017-04-12 09:14 PDT, Build Bot
no flags
Yusuke Suzuki
Comment 1 2017-04-12 07:36:23 PDT
Build Bot
Comment 2 2017-04-12 09:13:58 PDT
Comment on attachment 306909 [details] Patch Attachment 306909 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3524411 New failing tests: webrtc/negotiatedneeded-event-addStream.html
Build Bot
Comment 3 2017-04-12 09:14:00 PDT
Created attachment 306914 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Mark Lam
Comment 4 2017-04-12 09:38:58 PDT
Comment on attachment 306909 [details] 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.
Yusuke Suzuki
Comment 5 2017-04-12 09:53:58 PDT
Comment on attachment 306909 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306909&action=review Thanks! >> 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? OK, let's drop OS(DARWIN) here. >> Source/JavaScriptCore/heap/MachineStackMarker.cpp:244 >> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT) > > Ditto. Fixed. >> Source/JavaScriptCore/heap/MachineStackMarker.cpp:254 >> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT) > > Ditto. Fixed.
Yusuke Suzuki
Comment 6 2017-04-12 09:59:39 PDT
Yusuke Suzuki
Comment 7 2017-04-12 10:20:06 PDT
Note You need to log in before you can comment on or make changes to this bug.