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+

Description Mark Lam 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.
Comment 1 Mark Lam 2013-12-27 10:30:05 PST
Created attachment 220064 [details]
the patch.
Comment 2 Mark Lam 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.
Comment 3 Mark Lam 2013-12-27 10:36:32 PST
Landed in r161104: <http://trac.webkit.org/r161104>.
Comment 4 Geoffrey Garen 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++".
Comment 5 Mark Lam 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.
Comment 6 Mark Lam 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.
Comment 7 Geoffrey Garen 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.
Comment 8 Geoffrey Garen 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
Comment 9 Mark Lam 2014-01-02 16:34:16 PST
Created attachment 220267 [details]
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope.
Comment 10 Geoffrey Garen 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
Comment 11 Mark Lam 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>.