I'll address other comments in a separate CL.
On 2014/12/01 15:14:07, arv wrote:
I thought the design we agreed upon was to allow statements and
expressions
that
did not reference `this`?
I can change this to work in that fashion now that I am back.
The proper design would be to use a TDZ for this. I do not think we should
expand purely syntactic restriction to more than one statement
(e.g. what about
if (1) { super(); }
or
{ try { super(); } }
?
https://codereview.chromium.org/766663003/diff/20001/src/ast-this-access-visitor.cc
File src/ast-this-access-visitor.cc (right):
https://codereview.chromium.org/766663003/diff/20001/src/ast-this-access-visitor.cc#newcode55
src/ast-this-access-visitor.cc:55: void ATAV::VisitBlock(Block* stmt) {
VisitStatements(stmt->statements()); }
I'm surprised that we don't have a base class that does all of this
already?
Base recursive visitors are considered harmful. If you have to spell all
cases
in all visitors, then you have to think about every visitor when you add an
AST
node.
If there is a base recursive visitor, it is easy to forget to update some of
them.
https://codereview.chromium.org/766663003/diff/20001/src/ast-this-access-visitor.cc#newcode98
src/ast-this-access-visitor.cc:98: VisitIfNotNull(e->constructor());
constructor cannot be null. If there isn't a user defined constructor we
create
the default constructor.
https://codereview.chromium.org/766663003/diff/20001/src/ast-this-access-visitor.h
File src/ast-this-access-visitor.h (right):
https://codereview.chromium.org/766663003/diff/20001/src/ast-this-access-visitor.h#newcode33
src/ast-this-access-visitor.h:33: } // namespace v8::intrenal
typo
https://codereview.chromium.org/766663003/diff/20001/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/766663003/diff/20001/src/compiler.cc#newcode772
src/compiler.cc:772: MessageLocation location(info->script(),
lit->start_position(),
This position is not all that good.
https://codereview.chromium.org/766663003/diff/20001/src/messages.js
File src/messages.js (right):
https://codereview.chromium.org/766663003/diff/20001/src/messages.js#newcode186
src/messages.js:186: super_constructor_call: ["A 'super'
constructor
call
may only appear as the first statement of a function, and its arguments
may
not
access 'this'. Other forms are not yet supported."]
I think you fixed the duplicate property in a follow up CL?
Yes.
https://codereview.chromium.org/766663003/diff/20001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/766663003/diff/20001/src/scopes.cc#newcode905
src/scopes.cc:905: if (scope_uses_super_constructor_call_) {
Inconsistent use of {}
https://codereview.chromium.org/766663003/diff/20001/test/mjsunit/harmony/classes.js
File test/mjsunit/harmony/classes.js (right):
https://codereview.chromium.org/766663003/diff/20001/test/mjsunit/harmony/classes.js#newcode786
test/mjsunit/harmony/classes.js:786: var y;
Why should this throw? I thought the design we agreed upon was to allow
statements and expressions that did not reference `this`?
https://codereview.chromium.org/766663003/diff/20001/test/mjsunit/harmony/classes.js#newcode851
test/mjsunit/harmony/classes.js:851: }());
Can you add a test for `super.method()` too?
class C extends B {
constructor() {
super.method();
super();
}
}
https://codereview.chromium.org/766663003/
--
--
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.
