thanks for review!
https://codereview.chromium.org/943543002/diff/420001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/943543002/diff/420001/src/parser.cc#newcode2240
src/parser.cc:2240: DCHECK(var != nullptr);
On 2015/02/25 16:37:32, arv wrote:
DCHECK_NOT_NULL
Done.
https://codereview.chromium.org/943543002/diff/420001/src/parser.cc#newcode2298
src/parser.cc:2298: // Record the end position of the initializer.
On 2015/02/25 16:37:32, arv wrote:
Maybe move this comment to before the if since it is same for both
branches.
Hmm, that's kinda far away... instead, I made these comments more useful
by clarifying why the end positions are different in these 2 cases.
https://codereview.chromium.org/943543002/diff/420001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/943543002/diff/420001/src/scopes.cc#newcode1081
src/scopes.cc:1081: // ones. TODO(marja,rossberg): Detect errors across
different evals
On 2015/02/25 16:37:32, arv wrote:
Move TODO to new line?
Done.
https://codereview.chromium.org/943543002/diff/420001/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/943543002/diff/420001/src/scopes.h#newcode134
src/scopes.h:134: InitializationFlag init_flag, bool is_function,
On 2015/02/25 16:37:32, arv wrote:
Maybe use Variable::Kind here instead of the boolean?
Done.
https://codereview.chromium.org/943543002/diff/420001/test/mjsunit/strong/declaration-after-use.js
File test/mjsunit/strong/declaration-after-use.js (right):
https://codereview.chromium.org/943543002/diff/420001/test/mjsunit/strong/declaration-after-use.js#newcode82
test/mjsunit/strong/declaration-after-use.js:82: })();
On 2015/02/25 16:37:32, arv wrote:
Maybe test for-of loops too?
Not sure what cases you meant exactly; I thought maybe:
'use strong'; if (false) { for (let x of x) { } }
But that is not an error because the latter x resolves to the global
scope. For example this works:
"use strict"; var x = [1,2]; for (let x of x) { print(x); }
https://codereview.chromium.org/943543002/diff/420001/test/mjsunit/strong/declaration-after-use.js#newcode100
test/mjsunit/strong/declaration-after-use.js:100: "'use strong'; if
(false) { let C = class C2 { method() { C; } } }",
On 2015/02/25 16:49:18, arv wrote:
On 2015/02/25 16:37:32, arv wrote:
> This is kind of weird.
>
> What about?
>
> let f = function g() {
> g; // Error or not?
> };
I realized I misread this one.
I see you already have test for:
(class C { constructor() { C; } });
but I don't see anything for:
(function f() { f; });
Isn't the func1 test below testing this sufficiently:
function func1() { func1; this; }
https://codereview.chromium.org/943543002/diff/420001/test/mjsunit/strong/declaration-after-use.js#newcode104
test/mjsunit/strong/declaration-after-use.js:104: // tests.
On 2015/02/25 16:37:32, arv wrote:
I'm surprised that doesn't work already?
The basic case works (added it above):
'use strong'; if (false) { let o = { 'a': 'b', [o.a]: 'c'}; }
And one case for referencing a let declared class in a computed property
name:
'use strong'; if (false) {
let C = class C2 {
static a() { return 'A'; }
[C.a()]() { return 'B'; } // C shouldn't be allowed here
};
}
rossberg@ thought there are prob some corner cases where we don't detect
errors w/ the class name even though we should, thus the TODO to sort
that out later.
https://codereview.chromium.org/943543002/
--
--
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.
