https://codereview.chromium.org/366153002/diff/900001/include/v8.h
File include/v8.h (right):
https://codereview.chromium.org/366153002/diff/900001/include/v8.h#newcode1111
include/v8.h:1111: virtual unsigned GetMoreData(const char** src) = 0;
I think size_t is more appropriate than unsigned.
https://codereview.chromium.org/366153002/diff/900001/include/v8.h#newcode1118
include/v8.h:1118: void SetEncoding(Encoding e) { encoding = e; }
What's the rationale behind this two-stage initialization of the
encoding? Naively, I would assume a single constructor with the encoding
and "encoding" being const. If there is a good reason for the current
design, it should be explained in a comment.
https://codereview.chromium.org/366153002/diff/900001/include/v8.h#newcode1134
include/v8.h:1134: ~StreamedSource();
If we don't allow subclassing, we should mark this class as final. No
clue whether we want subclassing, though... :-)
https://codereview.chromium.org/366153002/diff/900001/include/v8.h#newcode1208
include/v8.h:1208: static ScriptStreamingTask* StartStreamingScript(
It might be nice to move this function and the one below to Isolate, but
this is not crucial. Nevertheless, "static" functions are a code smell
most of the time.
https://codereview.chromium.org/366153002/diff/900001/src/background-parsing-task.h
File src/background-parsing-task.h (right):
https://codereview.chromium.org/366153002/diff/900001/src/background-parsing-task.h#newcode23
src/background-parsing-task.h:23: isolate_(isolate) {}
Why do we remember the Isolate? Just passing the hash seed would result
in less coupling.
https://codereview.chromium.org/366153002/diff/900001/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/366153002/diff/900001/src/compiler.cc#newcode862
src/compiler.cc:862: // produced
Funny spacing.
https://codereview.chromium.org/366153002/diff/900001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):
https://codereview.chromium.org/366153002/diff/900001/test/cctest/test-api.cc#newcode23016
test/cctest/test-api.cc:23016: unsigned len = strlen(chunks_[index_]);
size_t
https://codereview.chromium.org/366153002/diff/900001/test/cctest/test-api.cc#newcode23017
test/cctest/test-api.cc:23017: // snprintf will NULL-terminate, so
reserve space for the NULL too. However,
Nit: It zero-terminates, not NULL-terminates. :-) And even that is not
guaranteed. And some VisualStudio versions don't have it.
https://codereview.chromium.org/366153002/diff/900001/test/cctest/test-api.cc#newcode23021
test/cctest/test-api.cc:23021: snprintf(copy, len + 1, "%s",
chunks_[index_]);
Wouldn't memcpy simplify things here and below?
https://codereview.chromium.org/366153002/
--
--
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.
