| Summary: | CStack: Cosmetic: rename VM::entryScope to firstEntryScope. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
| Component: | JavaScriptCore | Assignee: | 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
Mark Lam
2013-12-27 10:25:47 PST
Created attachment 220064 [details]
the patch.
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. Landed in r161104: <http://trac.webkit.org/r161104>. Why? "Previous first" sounds like an oxymoron. "Entry scope" makes more sense to me. "Entry" means "entry from C++". (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. (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. > 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 on attachment 220064 [details]
the patch.
I see. I'll mark this patch r+, since it's a
Created attachment 220267 [details]
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope.
Comment on attachment 220267 [details]
follow up patch to remove the need for VMEntryScope::m_prevFirstEntryScope.
r=me
Thanks for the review. Follow up patch landed in r161369 on the jsCStack branch: <http://trac.webkit.org/r161369>. |