LGTM with nits
https://codereview.chromium.org/882973002/diff/60001/src/compiler/graph-visualizer.h
File src/compiler/graph-visualizer.h (right):
https://codereview.chromium.org/882973002/diff/60001/src/compiler/graph-visualizer.h#newcode33
src/compiler/graph-visualizer.h:33: explicit AsJSON(const Graph& g,
SourcePositionTable* p)
Nit: Remove explicit.
https://codereview.chromium.org/882973002/diff/60001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):
https://codereview.chromium.org/882973002/diff/60001/src/compiler/pipeline.cc#newcode697
src/compiler/pipeline.cc:697: Vector<char> filename(buffer,
sizeof(buffer));
Using EmbeddedVector is simpler here.
https://codereview.chromium.org/882973002/diff/60001/src/compiler/pipeline.cc#newcode713
src/compiler/pipeline.cc:713: Vector<char>
full_filename(full_filename_buffer,
EmbeddedVector again.
https://codereview.chromium.org/882973002/diff/60001/src/ostreams.cc
File src/ostreams.cc (right):
https://codereview.chromium.org/882973002/diff/60001/src/ostreams.cc#newcode70
src/ostreams.cc:70: if (c.value == '\n') {
I think a 'comb-like' pattern plus inserting strings directly is a bit
clearer:
if (c.value == '\n') return os << "\\n";
if (c.value == '\r') return os << "\\r";
...
Or perhaps even better:
if (strstr("\n\r\'\"", c.value)) return os << '\\' << c.value;
...
https://codereview.chromium.org/882973002/
--
--
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.
