LGTM, only nits

https://codereview.chromium.org/881623002/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1273
src/parser.cc:1273: Module* Parser::ParseModuleVariable(bool* ok) {
Is this still used?

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1347
src/parser.cc:1347: // TODO(ES6): Add a let declaration for each name
Nit: misleading comment, as imports cannot be let-declarations; they are
a new form of declaration with a binding semantics completely different
from anything that exists in the language or implementation so far.

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode2311
src/parser.cc:2311: // Also detect attempts at 'let' declarations in
sloppy mode.
Nit: remove the "also"

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

https://codereview.chromium.org/881623002/diff/1/test/cctest/test-parsing.cc#newcode4722
test/cctest/test-parsing.cc:4722: CHECK(!parser.Parse());
On 2015/01/27 01:58:37, adamk wrote:
On 2015/01/27 01:17:17, arv wrote:
> Would it not be cleaner if we added ParseScript and ParseModule?

This was extracted from a branch where I had the whole compilation
pipeline
parameterized. The trouble is there's lots of code to be shared
between the two
paths (even in the parser, as you see, it's mostly shared), so adding
a bit to
CompilationInfo seems more readable than passing a bool "is_module" to
a bunch
of helper methods. And definitely more clear than duplicating the code
around
the edges.

Put another way: what code would ParseModule and ParseScript methods
on the
parser contain?

I don't think we have a separate ParseEval either, so the current CL is
at least consistent.

https://codereview.chromium.org/881623002/

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