Fix creating substring in string.replace(<global regexp>, <function>).

src/runtime.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 3929 matching lines @@
         match_end = current_match[1];
 
         {
           // Avoid accumulating new handles inside loop.
           HandleScope temp_scope(isolate);
           // Arguments array to replace function is match, captures, index and
           // subject, i.e., 3 + capture count in total.
           Handle<FixedArray> elements =
               isolate->factory()->NewFixedArray(3 + capture_count);
           Handle<String> match;
           if (!first) {

[Lasse Reichstein Nielsen, 2012/05/29 09:06:52]

"first" is actually badly named. It should be "atStart".

             match = isolate->factory()->NewProperSubString(subject,
                                                            match_start,
                                                            match_end);
           } else {
             match = isolate->factory()->NewSubString(subject,
                                                      match_start,
                                                      match_end);
-            first = false;
           }
           elements->set(0, *match);
           for (int i = 1; i <= capture_count; i++) {
             int start = current_match[i * 2];
             if (start >= 0) {
               int end = current_match[i * 2 + 1];
               ASSERT(start <= end);
-              Handle<String> substring =
-                  isolate->factory()->NewProperSubString(subject, start, end);
+              Handle<String> substring;
+              if (!first) {

[Lasse Reichstein Nielsen, 2012/05/29 09:06:52]

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.

+                substring =
+                    isolate->factory()->NewProperSubString(subject, start, end);
+              } else {
+                substring =
+                    isolate->factory()->NewSubString(subject, start, end);
+              }
               elements->set(i, *substring);
             } else {
               ASSERT(current_match[i * 2 + 1] < 0);
               elements->set(i, isolate->heap()->undefined_value());
             }
           }
           elements->set(capture_count + 1, Smi::FromInt(match_start));
           elements->set(capture_count + 2, *subject);
           builder->Add(*isolate->factory()->NewJSArrayWithElements(elements));
         }
+        first = false;
       }
 
       // If we did not get the maximum number of matches, we can stop here
       // since there are no matches left.
       if (num_matches < max_matches) break;
 
       if (match_end > match_start) {
         pos = match_end;
       } else {
         pos = match_end + 1;
@@ skipping 9551 matching lines @@
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     isolate->counters()->gc_last_resort_from_js()->Increment();
     isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags,
                                        "Runtime::PerformGC");
   }
 }
 
 
 } }  // namespace v8::internal