Looks good, just a few nits and some more tests.
https://codereview.chromium.org/663683006/diff/600001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/663683006/diff/600001/src/parser.cc#newcode5054
src/parser.cc:5054: #define COOKED_STRING(i) cooked_strings->at(i)
Nit: please remove these macros, they don't seem worth it.
https://codereview.chromium.org/663683006/diff/600001/src/parser.cc#newcode5077
src/parser.cc:5077: EXPR(i), POS(i)), POS(i));
Nit: indentation?
https://codereview.chromium.org/663683006/diff/600001/src/parser.cc#newcode5098
src/parser.cc:5098: Expression* expr = factory()->NewCallRuntime(
Nit: rename from 'expr' to 'call_site'
https://codereview.chromium.org/663683006/diff/600001/src/parser.cc#newcode5150
src/parser.cc:5150: if (raw_chars[from_index + 1] == '\n') {
Perhaps add a comment explaining that it is actually safe to access
index+1 here without checking against the length first.
https://codereview.chromium.org/663683006/diff/600001/src/parser.cc#newcode5153
src/parser.cc:5153: ch = '\n';
Nit: put this first.
https://codereview.chromium.org/663683006/diff/600001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/663683006/diff/600001/src/preparser.h#newcode2925
src/preparser.h:2925: // If we reach a TEMPLATE_TAIL first, we are
parsing a NoSubstitutionTemplate.
Nit: put this alternative into a conditional instead of the other, since
it is much shorter.
https://codereview.chromium.org/663683006/diff/600001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
https://codereview.chromium.org/663683006/diff/600001/test/cctest/test-parsing.cc#newcode4348
test/cctest/test-parsing.cc:4348: }
Add tests for some of the different InputElementTemplateTail
alternatives, e.g.
"`foo${a /* comment */}bar`"
"`foo${a \n}bar`"
"`foo${a // comment\n}bar`"
etc.
https://codereview.chromium.org/663683006/diff/600001/test/cctest/test-parsing.cc#newcode4397
test/cctest/test-parsing.cc:4397: }
We should also have sync test cases where the script ends in the middle
of a substitution, e.g. "`foo${a", or where a substitution expression is
otherwise ill-formed, e.g., "`foo${5 if}bar`" or "`foo${f(}bar`".
https://codereview.chromium.org/663683006/
--
--
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.
