http://codereview.chromium.org/1594017/diff/8001/9002 File src/api.cc (right):
http://codereview.chromium.org/1594017/diff/8001/9002#newcode2731 src/api.cc:2731: EnsureInitialized("v8::String::Flatten()"); I'd add ON_BAILOUT as well (or IsDeadCheck) as this operation has side-effects, not a pure query. http://codereview.chromium.org/1594017/diff/8001/9003 File test/cctest/test-strings.cc (right): http://codereview.chromium.org/1594017/diff/8001/9003#newcode357 test/cctest/test-strings.cc:357: v8::Local<v8::String> cons = v8::String::Concat(a,b); I'd suspect that would create a flat string: v8::String::Concat delegates to v8::internal::Factory::NewConsString which calls Heap::AllocateConsString. This function (https://cs.corp.google.com/p#chrome/trunk/src/v8/src/heap.cc&q=AllocateConsString&sa=N&cd=2&ct=rc) would flatten result if its length is less than String::kMinNonFlatLength which is currently 13 (see objects.h). So you should probably pick longer strings and maybe assert that result string is cons. http://codereview.chromium.org/1594017/diff/8001/9003#newcode364 test/cctest/test-strings.cc:364: for (i = 0; i < 3; i++) by no mean insisting, but as you don't reuse i after first for-loop, I'd rather wrote it for (int i = ...). Up to you. http://codereview.chromium.org/1594017/diff/8001/9003#newcode365 test/cctest/test-strings.cc:365: CHECK_EQ(stringA[i], buffer[i]); nit: current v8 styles is to either keep it in one line---for (...) CHECK_EQ(...) or put into block: for (...) { CHECK_EQ(...); } http://codereview.chromium.org/1594017/diff/8001/9003#newcode368 test/cctest/test-strings.cc:368: CHECK_EQ(stringB[i], buffer[i+3]); ditto http://codereview.chromium.org/1594017 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev To unsubscribe, reply using "remove me" as the subject.
