drive-by-comment

https://chromiumcodereview.appspot.com/10454032/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/10454032/diff/1/src/runtime.cc#newcode3950
src/runtime.cc:3950: if (!first) {
"first" is actually badly named. It should be "atStart".

https://chromiumcodereview.appspot.com/10454032/diff/1/src/runtime.cc#newcode3966
src/runtime.cc:3966: if (!first) {
Perhaps drop "first" and just check if start > 0 here.
Or, alternatively, just call NewSubString every time. I bet it starts
checking whether  start == 0  anyway, so removing the test here makes up
for that.

https://chromiumcodereview.appspot.com/10454032/diff/1/test/mjsunit/regexp-global.js
File test/mjsunit/regexp-global.js (right):

https://chromiumcodereview.appspot.com/10454032/diff/1/test/mjsunit/regexp-global.js#newcode129
test/mjsunit/regexp-global.js:129: // Test capture that is a real
substring.
Description isn't very descriptive :)
You are testing that a regexp that matches the empty string will match
once at the end of the string?

https://chromiumcodereview.appspot.com/10454032/diff/1/test/mjsunit/regexp-global.js#newcode132
test/mjsunit/regexp-global.js:132: assertEquals("~~");
Shouldn't there be a second argument here, probably str?
If this doesn't fail, is there a bug in assertEquals?

https://chromiumcodereview.appspot.com/10454032/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to