Very nice.

https://codereview.chromium.org/663683006/diff/320001/src/harmony-templates.js
File src/harmony-templates.js (right):

https://codereview.chromium.org/663683006/diff/320001/src/harmony-templates.js#newcode14
src/harmony-templates.js:14: for (; index < count; ++index) {
why not?

for (var index = 0; index < count; ++index) {

https://codereview.chromium.org/663683006/diff/320001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/663683006/diff/320001/src/parser.cc#newcode5097
src/parser.cc:5097: ALLOW_NULLS, FAST_STRING_TRAVERSAL, offset+1,
length-1, &length);
nit: ws around + and -

https://codereview.chromium.org/663683006/diff/320001/src/parser.cc#newcode5130
src/parser.cc:5130: ast_value_factory()->get_template_callsite_string(),
NULL, args, start);
I'm not sure but isn't it better to use the function id here?

https://codereview.chromium.org/663683006/diff/320001/src/parser.cc#newcode5138
src/parser.cc:5138: if (fni_ != NULL) fni_->RemoveLastFunction();
I don't see where the function was added?

https://codereview.chromium.org/663683006/diff/320001/src/parser.h
File src/parser.h (right):

https://codereview.chromium.org/663683006/diff/320001/src/parser.h#newcode976
src/parser.h:976: ParserTraits::TemplateLiteralState
ParserTraits::OpenTemplateLiteral(int pos) {
nit: 2 empty lines between functions

https://codereview.chromium.org/663683006/diff/320001/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/663683006/diff/320001/src/preparser.h#newcode2910
src/preparser.h:2910: DCHECK(false);
UNREACHABLE();

But maybe change it to:

DCHECK_EQ(next, Token::TEMPLATE_TAIL);
// Once we've reached a TEMPLATE_TAIL, we can close the TemplateLiteral.
return Traits::CloseTemplateLiteral(&ts, start, tag);

https://codereview.chromium.org/663683006/diff/320001/src/scanner.cc
File src/scanner.cc (right):

https://codereview.chromium.org/663683006/diff/320001/src/scanner.cc#newcode683
src/scanner.cc:683: bool singleCharEscape = true;
This is no longer used

https://codereview.chromium.org/663683006/diff/320001/test/mjsunit/es6/templates.js
File test/mjsunit/es6/templates.js (right):

https://codereview.chromium.org/663683006/diff/320001/test/mjsunit/es6/templates.js#newcode7
test/mjsunit/es6/templates.js:7: var num = 5;
I prefer writing these tests in functions. Makes it easier to read.

(function TestBasicExpressions() {
  ...
})();


(function TestExpressionsContainingTemplates() {
  ...
})();


(function TestTaggedTemplates() {
  ...
})();


...

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.

Reply via email to