Bug 126266 – CStack: Cosmetic: rename VM::entryScope to firstEntryScope.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

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.

[Mark Lam]

Created attachment 220064 [details]
the patch.

[Mark Lam]

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]

Landed in r161104: <http://trac.webkit.org/r161104>.

[Geoffrey Garen]

Why?

"Previous first" sounds like an oxymoron. "Entry scope" makes more sense to me. "Entry" means "entry from C++".

[Mark Lam]

(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]

(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]

> 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 on attachment 220064 [details]
the patch.

I see. I'll mark this patch r+, since it's a

[Mark Lam]

Created attachment 220267 [details]
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope.

[Geoffrey Garen]

Comment on attachment 220267 [details]
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope.

r=me

[Mark Lam]

Thanks for the review.  Follow up patch landed in r161369 on the jsCStack branch: <http://trac.webkit.org/r161369>.