Bug 206970

Summary: Don't include VM.h in CallFrame.h
Product: WebKit Reporter: Robin Morisset <rmorisset>
Component: JavaScriptCoreAssignee: Robin Morisset <rmorisset>
Status: NEW ---    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
saam: review+
Patch
none
Patch
none
Patch none

Description Robin Morisset 2020-01-29 14:57:01 PST
CallFrame.h is pretty small, and is included in lots of places. It currently includes VM.h which is huge and brings in a ton of code, all just for DECLARE_CALL_FRAME, which takes vm as argument.
This macro could as easily be declared in VM.h, and it significantly help compile times for all the files which don't use it (and so no longer have to depend on VM.h)
Comment 1 Robin Morisset 2020-01-29 14:58:41 PST
Created attachment 389192 [details]
Patch
Comment 2 Saam Barati 2020-01-29 15:00:16 PST
Comment on attachment 389192 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:9
> +        This macro could as easily be declared in VM.h, and it significantly help compile times for all the files which don't use it (and so no longer have to depend on VM.h)

"help" => "helps"
Comment 3 Mark Lam 2020-01-29 15:12:12 PST
Comment on attachment 389192 [details]
Patch

Can you add a comment next to the DECLARE_CALL_FRAME macro indicating that it was moved here from CallFrame.h so that CallFrame.h doesn’t have to include VM.h, and that this is needed to reduce build time.
Comment 4 Robin Morisset 2020-02-03 18:59:22 PST
Created attachment 389614 [details]
Patch

Add a comment per Mark's suggestion, and add a missing '#include "CPU.h"' which was causing failures in the 32-bit x86 EWS bot.
Comment 5 Robin Morisset 2020-02-05 13:14:56 PST
Created attachment 389857 [details]
Patch

rebase
Comment 6 Robin Morisset 2020-02-05 15:30:50 PST
Created attachment 389885 [details]
Patch