Bug 126266 - CStack: Cosmetic: rename VM::entryScope to firstEntryScope.
Summary: CStack: Cosmetic: rename VM::entryScope to firstEntryScope.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks: 126320
  Show dependency tree / graph
 
Reported: 2013-12-27 10:25 PST by Mark Lam
Modified: 2014-01-09 13:08 PST (History)
5 users (show)

See Also:

Attachments
the patch. (7.08 KB, patch)
2013-12-27 10:30 PST, Mark Lam 		ggaren: review-
Details | Formatted Diff | Diff
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+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>.