lgtm
https://codereview.chromium.org/490173002/diff/80001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/490173002/diff/80001/src/parser.cc#newcode751
src/parser.cc:751: *(info->ast_node_id_gen()) = AstNode::IdGen(0);
On 2014/08/22 07:59:07, marja wrote:
On 2014/08/21 13:43:35, rossberg wrote:
> Isn't this default constructed in the CompilationInfo class? Are we
reusing
comp
> infos in weird ways?
This would only change things if we parse with the same
CompilationInfo twice.
That shouldn't happen in the current state. I was trying to be overly
defensive
(in case somebody wants to parse the same compilationinfo again in the
future)
and not change the behavior in that case. Removed this line.
Right, I don't think we should ever do such reuse. I'm pretty sure more
things would break anyway.
https://codereview.chromium.org/490173002/diff/100001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/490173002/diff/100001/src/ast.h#newcode188
src/ast.h:188: explicit IdGen(int id) : id_(id) {}
Nit: could also merge the two constructors into one with default
argument 0.
https://codereview.chromium.org/490173002/diff/100001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/490173002/diff/100001/src/parser.cc#newcode349
src/parser.cc:349: saved_ast_node_id_gen_ =
*(parser_->ast_node_id_gen_);
Nit: some places left with redundant parens
https://codereview.chromium.org/490173002/
--
--
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.
