Bug 147657 - Replace JSFunctionNameScope with JSLexicalEnvironment for the function name scope.
Summary: Replace JSFunctionNameScope with JSLexicalEnvironment for the function name s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks: 147666
  Show dependency tree / graph
 
Reported: 2015-08-04 15:35 PDT by Saam Barati
Modified: 2015-08-05 11:30 PDT (History)
9 users (show)

See Also:

Attachments
patch (61.17 KB, patch)
2015-08-04 19:05 PDT, Saam Barati 		mark.lam: review+
mark.lam: commit-queue-
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 Saam Barati 2015-08-04 15:35:00 PDT 
We can just implement this as a "const"-like scope :)
Comment 1 Saam Barati 2015-08-04 19:05:51 PDT 
Created attachment 258257 [details]
patch
Comment 2 Mark Lam 2015-08-05 09:44:43 PDT 
Comment on attachment 258257 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258257&action=review

r=me with fixes.

Do we already have test coverage for all the aspects of the function name scope behavior?  If not, please add the missing pieces.

> Source/JavaScriptCore/ChangeLog:14
> +        this variable like a "const" because 

You're cut off here mid thought.
Comment 3 Saam Barati 2015-08-05 11:16:44 PDT 
(In reply to comment #2)
> Comment on attachment 258257 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258257&action=review
> 
> r=me with fixes.
> 
> Do we already have test coverage for all the aspects of the function name
> scope behavior?  If not, please add the missing pieces.
> 
We do have tests. I broke them and then fixed them :)

> > Source/JavaScriptCore/ChangeLog:14
> > +        this variable like a "const" because 
> 
> You're cut off here mid thought.
Fixed.
Comment 4 Saam Barati 2015-08-05 11:30:20 PDT 
landed in:
http://trac.webkit.org/changeset/187969