On Mon, Dec 1, 2014 at 10:37 AM, <[email protected]> wrote: > 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(); } } > ? >
That is a good point. Lets stick to this restricted version for now. > > > > 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. > -- erik -- -- 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.
