Bug 74628 – Avoid double-lookup when setting static-scoped vars in strict mode

• Status: RESOLVED
• Resolution: INVALID
• Component: JavaScriptCore

[Andy Wingo]

The patch to be attached avoids a double-lookup when setting variables bound in a static scope.

[Andy Wingo]

Created attachment 119462 [details]
Patch

[Andy Wingo]

Comment on attachment 119462 [details]
Patch

I'm looking to use this object for implementing heap-allocated block-scoped variables in the future, and this double-lookup would be more egregious in that case.

[Andy Wingo]

Created attachment 119463 [details]
Patch

[Andy Wingo]

Comment on attachment 119463 [details]
Patch

Fixed a style issue in the log.

[WebKit Review Bot]

Attachment 119462 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/ChangeLog:8:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andy Wingo]

Created attachment 122958 [details]
Patch

[Gavin Barraclough]

Hi Andy,

You really need to address my comments in https://bugs.webkit.org/show_bug.cgi?id=74633.

If the route we want to pursue to implement let & const is based on activations, then increasing coupling between the JSStaticScopeObject and JSVariableObject classes doesn't make sense.

[Andy Wingo]

(In reply to comment #7)
> Hi Andy,
> 
> You really need to address my comments in https://bugs.webkit.org/show_bug.cgi?id=74633.
> 
> If the route we want to pursue to implement let & const is based on activations, then increasing coupling between the JSStaticScopeObject and JSVariableObject classes doesn't make sense.

Is squirrelfish-dev a suitable venue for you?  I thought we might need a central location, given the proliferation of these bugs...

[Filip Pizlo]

Comment on attachment 122958 [details]
Patch

Is this still relevant?  Looks like this would need a fairly big rebase.

[Andy Wingo]

(In reply to comment #9)
> (From update of attachment 122958 [details])
> Is this still relevant?  Looks like this would need a fairly big rebase.

No idea.  I reckon anyone working in this area in the future would catch whatever bugs need to be fixed, so holding this open doesn't do anyone any good; closing.

[Csaba Osztrogonác]

Comment on attachment 122958 [details]
Patch

Cleared review? from attachment 122958 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).