Bug 126266

Summary: CStack: Cosmetic: rename VM::entryScope to firstEntryScope.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, msaboff, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 126320    
Attachments:
Description Flags
the patch.
ggaren: review-
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope. ggaren: review+

Mark Lam
Reported 2013-12-27 10:25:47 PST
This is a purely cosmetic change in preparation for an upcoming patch to have a separate stack limit for JS execution vs native C execution.
Attachments
the patch. (7.08 KB, patch)
2013-12-27 10:30 PST, Mark Lam 		ggaren: review-
DetailsFormatted DiffDiff
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope. (2.73 KB, patch)
2014-01-02 16:34 PST, Mark Lam 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2013-12-27 10:30:05 PST
Created attachment 220064 [details] the patch.
Mark Lam
Comment 2 2013-12-27 10:35:59 PST
Comment on attachment 220064 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=220064&action=review > Source/JavaScriptCore/ChangeLog:3 > + Stack: Cosmetic: rename VM::entryScope to firstEntryScope. typo: "Stack" ==> "CStack". Fixed before commit.
Mark Lam
Comment 3 2013-12-27 10:36:32 PST
Landed in r161104: <http://trac.webkit.org/r161104>.
Geoffrey Garen
Comment 4 2014-01-02 12:52:56 PST
Why? "Previous first" sounds like an oxymoron. "Entry scope" makes more sense to me. "Entry" means "entry from C++".
Mark Lam
Comment 5 2014-01-02 15:33:46 PST
(In reply to comment #4) > Why? > > "Previous first" sounds like an oxymoron. "Entry scope" makes more sense to me. "Entry" means "entry from C++". I renamed entryScope to firstEntryScope to distinguish it from topEntryScope which is introduced in https://bugs.webkit.org/show_bug.cgi?id=126334. topEntryScope will eventually be needed in the computation of JS stack usage in https://bugs.webkit.org/show_bug.cgi?id=126320 (which is what started this series of patches). Also, bear in mind that there is a chain of entryScopes (or will be with the introduction of topEntryScope). The first entry sc That said, I agree that “previousFirstEntryScope” sounds ridiculous, and it also makes the code harder to understand. I’ll upload a follow up patch shortly that will improve that part.
Mark Lam
Comment 6 2014-01-02 15:34:54 PST
(In reply to comment #5) > Also, bear in mind that there is a chain of entryScopes (or will be with the introduction of topEntryScope). The first entry sc Incomplete thought ... I meant to say: The first entry scope is not the only entry scope. Hence, just calling it entryScope may not be accurate.
Geoffrey Garen
Comment 7 2014-01-02 15:43:43 PST
> I renamed entryScope to firstEntryScope to distinguish it from topEntryScope which is introduced in https://bugs.webkit.org/show_bug.cgi?id=126334. I see. Putting that comment in your ChangeLog would have explained things for me. Since this is a part of adding topEntryScope, and since I've marked that patch r-, I'll mark this patch r- too.
Geoffrey Garen
Comment 8 2014-01-02 15:43:50 PST
Comment on attachment 220064 [details] the patch. I see. I'll mark this patch r+, since it's a
Mark Lam
Comment 9 2014-01-02 16:34:16 PST
Created attachment 220267 [details] follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope.
Geoffrey Garen
Comment 10 2014-01-03 12:07:37 PST
Comment on attachment 220267 [details] follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope. r=me
Mark Lam
Comment 11 2014-01-06 13:45:02 PST
Thanks for the review. Follow up patch landed in r161369 on the jsCStack branch: <http://trac.webkit.org/r161369>.
Note You need to log in before you can comment on or make changes to this bug.