LGTM

https://codereview.chromium.org/948303004/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/948303004/diff/1/src/ast.h#newcode613
src/ast.h:613: const AstRawString* module_specifier() const { return
module_specifier_; }
On 2015/02/25 13:55:49, rossberg wrote:
Hm, is it necessary that the specifier is duplicated on every
importdecl node?
That doesn't make sense from an AST perspective, and the fact that you
then need
a setter doesn't make it better.

At the moment the actual import statement is not represented in the AST
so then I see why this is needed. I think this approach is fine (at
least for now).

https://codereview.chromium.org/948303004/diff/1/src/parser.h
File src/parser.h (right):

https://codereview.chromium.org/948303004/diff/1/src/parser.h#newcode716
src/parser.h:716: ZoneList<ImportDeclaration*>* ParseNamedImports(int
pos, bool* ok);
On 2015/02/25 13:55:49, rossberg wrote:
While you're at it, can you perhaps change the other above functions
to _return_
their lists as well? I don't see a good reason why they are created by
the
caller.

The one above has 2 lists as out params.

https://codereview.chromium.org/948303004/diff/1/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/948303004/diff/1/test/cctest/test-parsing.cc#newcode5246
test/cctest/test-parsing.cc:5246: i::ImportDeclaration* decl =
declarations->at(i)->AsImportDeclaration();
Maybe remove the loop and check x and z separately?

https://codereview.chromium.org/948303004/

--
--
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.

Reply via email to