https://codereview.chromium.org/1226103002/diff/40001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1226103002/diff/40001/src/preparser.h#newcode3769
src/preparser.h:3769: if (is_strict(language_mode()) ||
allow_harmony_sloppy()) {
On 2015/07/10 12:13:17, rossberg wrote:
On 2015/07/08 23:42:13, Dan Ehrenberg wrote:
> A tangent, but it looks like CheckConflictingVarDeclarations is
implemented
only
> in the parser, not the preparser, so the errors aren't quite as
early as we'd
> optimally like. Any particular reason for this? It looks like the
preparser
> maintains the whole scope thing anyway.
No, the preparser doesn't maintain scopes.
This is not entirely true. We do set up the scopes but we do not create
the bindings in them in the preparser.
https://codereview.chromium.org/1226103002/diff/40001/test/mjsunit/harmony/block-conflicts-sloppy.js
File test/mjsunit/harmony/block-conflicts-sloppy.js (right):
https://codereview.chromium.org/1226103002/diff/40001/test/mjsunit/harmony/block-conflicts-sloppy.js#newcode62
test/mjsunit/harmony/block-conflicts-sloppy.js:62: //
TestAll('Conflict', 'eval("' + s + '");');
On 2015/07/10 12:13:17, rossberg wrote:
On 2015/07/09 14:30:53, arv wrote:
> On 2015/07/08 23:42:13, Dan Ehrenberg wrote:
> > I don't see why these tests wouldn't make sense. Wouldn't there be
a
conflict
> in
> > the new eval-driven block scope, even in sloppy mode?
>
> See the bug.
>
> eval('let x; var x;')
>
> is more similar to
>
> eval('{ let x; var x; }')
>
> we are not creating the lexical block correctly for sloppy evals so
these
tests
> are not passing or the tests are not correct for sloppy mode.
I suppose
let x; eval('var x');
is an error in sloppy mode, but not in strict mode. That's a fairly
unfortunate
semantic deviation, actually. But why does this difference only pop up
in the
global scope tests?
The other tests wrap the string in a function or a block so the implicit
lexical scope is not relevant.
These tests should probably be changed to use Realm.eval in
TestGlobal, so that
we test the actual global scope, and there should be a separate
TestEval
(similarly for strict mode). Can you add a TODO for that?
I'll add a todo (and/or add a comment to the bug)
https://codereview.chromium.org/1226103002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
