LGTM with comments.

https://chromiumcodereview.appspot.com/10184004/diff/1/src/macros.py
File src/macros.py (right):

https://chromiumcodereview.appspot.com/10184004/diff/1/src/macros.py#newcode199
src/macros.py:199: macro LAST_SUBJECT(array) = ((array)[1]);
We can avoid duplication:
macro LAST_SUBJECT(array) = ((array)[LAST_SUBJECT_INDEX]);

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

https://chromiumcodereview.appspot.com/10184004/diff/1/src/runtime.cc#newcode2885
src/runtime.cc:2885: static void
SetLastMatchInfoNoCaptures(Handle<String> subject,
Why not just move the implementation here, instead of forward
declaration?

https://chromiumcodereview.appspot.com/10184004/diff/1/src/string.js
File src/string.js (right):

https://chromiumcodereview.appspot.com/10184004/diff/1/src/string.js#newcode246
src/string.js:246: // We use this hack to detect whether
StringReplaceRegExpWithString found
Long line

https://chromiumcodereview.appspot.com/10184004/diff/1/src/string.js#newcode249
src/string.js:249: lastMatchInfo[LAST_SUBJECT_INDEX] = 0;
This is sad, but I don't see a better way.

https://chromiumcodereview.appspot.com/10184004/diff/1/src/string.js#newcode461
src/string.js:461: for ( ; i < len; i++) {
Can we make i local in the loop?

https://chromiumcodereview.appspot.com/10184004/diff/1/src/string.js#newcode487
src/string.js:487: for ( ; i < len; i++) {
Can we make i local in the loop?

https://chromiumcodereview.appspot.com/10184004/

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

Reply via email to