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

Robin Morisset
Reported 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)
Attachments
Patch (6.74 KB, patch)
2020-01-29 14:58 PST, Robin Morisset 		saam: review+
DetailsFormatted DiffDiff
Patch (7.38 KB, patch)
2020-02-03 18:59 PST, Robin Morisset 		no flags
DetailsFormatted DiffDiff
Patch (34.86 KB, patch)
2020-02-05 13:14 PST, Robin Morisset 		no flags
DetailsFormatted DiffDiff
Patch (7.90 KB, patch)
2020-02-05 15:30 PST, Robin Morisset 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Robin Morisset
Comment 1 2020-01-29 14:58:41 PST
Created attachment 389192 [details] Patch
Saam Barati
Comment 2 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"
Mark Lam
Comment 3 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.
Robin Morisset
Comment 4 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.
Robin Morisset
Comment 5 2020-02-05 13:14:56 PST
Created attachment 389857 [details] Patch rebase
Robin Morisset
Comment 6 2020-02-05 15:30:50 PST
Created attachment 389885 [details] Patch
Note You need to log in before you can comment on or make changes to this bug.