Feedback addressed, sending to CQ

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)
On 2015/01/28 15:00:43, Sven Panne wrote:
Nit: Remove explicit.

Done.

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));
On 2015/01/28 15:00:43, Sven Panne wrote:
Using EmbeddedVector is simpler here.

Done.

https://codereview.chromium.org/882973002/diff/60001/src/compiler/pipeline.cc#newcode713
src/compiler/pipeline.cc:713: Vector<char>
full_filename(full_filename_buffer,
On 2015/01/28 15:00:43, Sven Panne wrote:
EmbeddedVector again.

Done.

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') {
On 2015/01/28 15:00:43, Sven Panne wrote:
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;
    ...

Done.

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.

Reply via email to