LGTM with nits.

https://codereview.chromium.org/14895019/diff/2001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/14895019/diff/2001/src/hydrogen-instructions.cc#newcode812
src/hydrogen-instructions.cc:812: HValue *value = OperandAt(i);
nit: "HValue* value"

...or just "OperandAt(i)->PrintNameTo(stream);"

https://codereview.chromium.org/14895019/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/14895019/diff/2001/src/hydrogen.cc#newcode5442
src/hydrogen.cc:5442: void HGraph::MarkLive(HValue *ref, HValue* instr,
nit: while you're here: "HValue* ref", and if all parameters in a
method/function declaration/definition don't fit on one line, they each
get their own, as in:
void HGraph::MarkLive(HValue* ref,
                      HValue* instr,
                      ZoneList<HValue*>* worklist) {

https://codereview.chromium.org/14895019/diff/2001/src/hydrogen.cc#newcode5451
src/hydrogen.cc:5451: ALLOW_HANDLE_DEREF(isolate(), "debug mode
printing");
I don't see any handle being dereferenced?

https://codereview.chromium.org/14895019/diff/2001/test/mjsunit/regress/regress-seqstrsetchar-ex1.js
File test/mjsunit/regress/regress-seqstrsetchar-ex1.js (right):

https://codereview.chromium.org/14895019/diff/2001/test/mjsunit/regress/regress-seqstrsetchar-ex1.js#newcode32
test/mjsunit/regress/regress-seqstrsetchar-ex1.js:32: return 311;
nit: indentation

https://codereview.chromium.org/14895019/diff/2001/test/mjsunit/regress/regress-seqstrsetchar-ex1.js#newcode36
test/mjsunit/regress/regress-seqstrsetchar-ex1.js:36: // does not
actually do the same calculation but exhibits untagging bug
nit: trailing full stop please

https://codereview.chromium.org/14895019/

--
--
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/groups/opt_out.


Reply via email to