LGTM

http://codereview.chromium.org/7616009/diff/1/src/contexts.cc
File src/contexts.cc (right):

http://codereview.chromium.org/7616009/diff/1/src/contexts.cc#newcode183
src/contexts.cc:183: case Variable::LET:
I was just trying to understand the comment above. If a variable is
assigned a fixed context slot, there can only be one instance of the
variable in the context. That doesn't seem to match the behavior of
let-inside-loop, since multiple instances can be alive at the same time.

On the other hand, this code doesn't seem to care about that, just that
we know whether the variable is read-only or not, so it's probably fine.

I believe the current consensus about for(let x = ...; ...; ...)... is
that it declares an implicit block around the for where the let lives
(but for(let x in ...) will use a new instance for each iteration).

http://codereview.chromium.org/7616009/diff/13014/test/mjsunit/harmony/block-let-declaration.js
File test/mjsunit/harmony/block-let-declaration.js (right):

http://codereview.chromium.org/7616009/diff/13014/test/mjsunit/harmony/block-let-declaration.js#newcode45
test/mjsunit/harmony/block-let-declaration.js:45:
Try checking the value of x and y here, just for sanity.

http://codereview.chromium.org/7616009/diff/13014/test/mjsunit/harmony/block-let-declaration.js#newcode51
test/mjsunit/harmony/block-let-declaration.js:51:
Notic that these code blocks are executed using a direct call to eval in
non-strict code - i.e., they use the declaration environment of its
surrounding code (which is inside a block).
If that eval turned into an indirect call (which it REALLY should), the
code would be global (and no longer throw?).

If you want to explicitly check eval code, wrap a call to eval in a
function instead of relying on the implementation of assertThrows.

http://codereview.chromium.org/7616009/diff/13014/test/mjsunit/harmony/block-scoping.js
File test/mjsunit/harmony/block-scoping.js (right):

http://codereview.chromium.org/7616009/diff/13014/test/mjsunit/harmony/block-scoping.js#newcode179
test/mjsunit/harmony/block-scoping.js:179: y = j + 10;
Should y not just always be 10?
And I can't see "j" defined anywhere.

http://codereview.chromium.org/7616009/diff/13014/test/mjsunit/harmony/block-scoping.js#newcode183
test/mjsunit/harmony/block-scoping.js:183: }
And you don't call f8 (which is why it could be bugged and not fail :).

http://codereview.chromium.org/7616009/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to