http://codereview.chromium.org/9231017/diff/1/src/runtime.cc File src/runtime.cc (right):
http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3292 src/runtime.cc:3292: for (int i = 0; i < 2; i++) { On 2012/01/17 13:50:24, Erik Corry wrote:
I think this would be clearer without the loop, just duplicating the
call. Done. http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3300 src/runtime.cc:3300: if (!result.is_null()) return *result; On 2012/01/17 13:51:21, Erik Corry wrote:
Should you have a test for found == false here and then return the
original
string?
Allocation is only ever done if found==true. In all other cases, either a null-handle or the original subject string is returned, so there is no need for an additional check. http://codereview.chromium.org/9231017/diff/1/test/mjsunit/string-replace-one-char.js File test/mjsunit/string-replace-one-char.js (right): http://codereview.chromium.org/9231017/diff/1/test/mjsunit/string-replace-one-char.js#newcode39 test/mjsunit/string-replace-one-char.js:39: "$", "###"); On 2012/01/17 13:50:24, Erik Corry wrote:
It would be nice to test with a replacement string like "x$0x" to see
that the
'$' detection is working.
Done. I don't know what I was thinking. http://codereview.chromium.org/9231017/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
