Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(482)

Issue 932283003: [strong] make function and class declarations lexical & immutable (Closed)

Created:
5 years, 10 months ago by rossberg
Modified:
5 years, 10 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[strong] make function and class declarations lexical & immutable R=arv@chromium.org BUG= Committed: https://crrev.com/7b49ed658cec317ad8009b08812f8aa42abebb71 Cr-Commit-Position: refs/heads/master@{#26755}

Patch Set 1 #

Patch Set 2 : Adjusted assertion #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -21 lines) Patch
M src/ast.h View 1 1 chunk +1 line, -2 lines 0 comments Download
M src/parser.cc View 2 chunks +6 lines, -3 lines 0 comments Download
D test/mjsunit/strong/arguments.js View 1 chunk +0 lines, -16 lines 0 comments Download
A test/mjsunit/strong/classes.js View 1 chunk +17 lines, -0 lines 0 comments Download
A + test/mjsunit/strong/functions.js View 2 chunks +17 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (5 generated)
rossberg
5 years, 10 months ago (2015-02-19 15:45:02 UTC) #1
arv (Not doing code reviews)
LGTM Nice. A lot simpler than I would have thought.
5 years, 10 months ago (2015-02-19 15:48:56 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/932283003/1
5 years, 10 months ago (2015-02-19 15:51:08 UTC) #4
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel/builds/3129)
5 years, 10 months ago (2015-02-19 16:05:10 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/932283003/20001
5 years, 10 months ago (2015-02-19 16:28:18 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 10 months ago (2015-02-19 16:49:19 UTC) #10
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/7b49ed658cec317ad8009b08812f8aa42abebb71 Cr-Commit-Position: refs/heads/master@{#26755}
5 years, 10 months ago (2015-02-19 16:49:33 UTC) #11
marja
I was staring at this change for a long time and I don't understand it. ...
5 years, 10 months ago (2015-02-20 17:08:55 UTC) #13
rossberg
5 years, 10 months ago (2015-02-23 12:48:37 UTC) #14
Message was sent while issue was closed.
On 2015/02/20 17:08:55, marja wrote:
> I was staring at this change for a long time and I don't understand it. Why
does
> it work? Because there's some other code somewhere which says that consts are
> not put to the global object?
> 
> How does const (or function) resolution across scripts work? Obviously it
works
> somehow since consts work :) (But how can we add a paranoid test that we don't
> break function resolution across scripts?)

The CONST and LET variable kinds already implement lexical scoping, and lexical
scoping works across script (covered by tests). Otherwise we wouldn't have
shipped that. :)  So this works out of the box -- it would have been rather
alarming if it didn't.

(Currently we also have CONST_LEGACY, which is not lexical, and puts bindings on
the global object instead. That is going to go away eventually.)

Powered by Google App Engine
This is Rietveld 408576698