https://codereview.chromium.org/376223002/diff/1/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/376223002/diff/1/src/api.cc#newcode1709
src/api.cc:1709: ASSERT(source->cached_data == NULL);
Shouldn't you crash if source->cached_data != NULL? Now we'll just
overwrite it silently. ASSERT -> CHECK?
https://codereview.chromium.org/376223002/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/376223002/diff/1/src/parser.cc#newcode217
src/parser.cc:217: if (HasError()) {
Afaics we'll never have an error in normal operation, just in tests. So
maybe you could just return false here?
https://codereview.chromium.org/376223002/diff/1/src/parser.cc#newcode259
src/parser.cc:259: int symbol_data_offset =
Omg, this is messier than I thought. We don't have symbol data any more!
Could you remove it since you're at it?
https://codereview.chromium.org/376223002/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):
https://codereview.chromium.org/376223002/diff/1/test/cctest/test-api.cc#newcode14833
test/cctest/test-api.cc:14833:
TEST(ExpectFailPreCompileDeserializationError) {
I'd just remove this test, it makes no sense to keep it as a failing
test.
If we cannot assert that a CHECK is hit, but we assert that the test
fails, then it rots very easily. E.g., it can start failing due to some
other reason than ScriptData, and nobody will notice. And after that
point, it doesn't test anything.
https://codereview.chromium.org/376223002/diff/1/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
https://codereview.chromium.org/376223002/diff/1/test/cctest/test-parsing.cc#newcode1260
test/cctest/test-parsing.cc:1260: if (!pd.HasError()) {
Alright, now I remember why I didn't remove the "ScriptData can have an
error" functionality; it's used in tests but it will never occur in
normal operation (we won't give the data to the embedder if the
compilation has failed).
https://codereview.chromium.org/376223002/
--
--
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.
