lgtm
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/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 a long-standing
issue. We would love to do that, but experiments showed that it is
fairly costly and causes significant regressions in e.g. CodeLoad. It's
still an open problem how to diagnose binding errors early in all cases,
and we have punted so far.
(It's a particular pain point for strong mode, btw.)
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/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?
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?
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.
