LGTM, module a few nits.
https://codereview.chromium.org/314603004/diff/1/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/314603004/diff/1/src/ast.h#newcode3156
src/ast.h:3156: Literal* NewLiteral(const AstString* string, int pos) {
On 2014/06/12 09:57:53, marja wrote:
On 2014/06/11 14:58:12, rossberg wrote:
> I think we should name all these `NewSomethingLiteral`.
Done. And I named the one which takes an AstValue*
"NewSpecialLiteral". (It's
used for null, undefined and the hole.)
I'd prefer to just have NewNull, NewUndefined, NewHole instead (cf.
comments below).
https://codereview.chromium.org/314603004/diff/140001/src/ast-value-factory.h
File src/ast-value-factory.h (right):
https://codereview.chromium.org/314603004/diff/140001/src/ast-value-factory.h#newcode188
src/ast-value-factory.h:188: type_(t) {}
Perhaps ASSERT that t in {NULL, UNDEFINED, HOLE}.
https://codereview.chromium.org/314603004/diff/140001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/314603004/diff/140001/src/parser.cc#newcode692
src/parser.cc:692: return factory->NewSpecialLiteral(
Nit: just for symmetry, I would wrap this up into a NewNullLiteral.
https://codereview.chromium.org/314603004/diff/140001/src/parser.cc#newcode734
src/parser.cc:734: return factory->NewSpecialLiteral(
...and similarly here.
https://codereview.chromium.org/314603004/diff/140001/src/parser.cc#newcode3673
src/parser.cc:3673: Expression* undefined =
factory()->NewSpecialLiteral(
...and here.
https://codereview.chromium.org/314603004/diff/140001/src/parser.cc#newcode3769
src/parser.cc:3769: return
factory()->NewSpecialLiteral(ast_value_factory_->NewUndefined(),
...and here.
https://codereview.chromium.org/314603004/
--
--
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.
