Reviewers: Mads Ager,

Description:
Make String.prototype.split honor limit when separator is empty.

BUG=929

Please review this at http://codereview.chromium.org/4750003/

Affected files:
  M src/runtime.h
  M src/runtime.cc
  M src/string.js
  M test/mjsunit/string-split.js


Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 5534db557ce53faee9c85aed9681749804354619..327114898078769f44d057252b850acc799ac182 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5020,11 +5020,12 @@ static int CopyCachedAsciiCharsToArray(const char* chars,
 // For example, "foo" => ["f", "o", "o"].
 static MaybeObject* Runtime_StringToArray(Arguments args) {
   HandleScope scope;
-  ASSERT(args.length() == 1);
+  ASSERT(args.length() == 2);
   CONVERT_ARG_CHECKED(String, s, 0);
+  CONVERT_NUMBER_CHECKED(uint32_t, limit, Uint32, args[1]);

   s->TryFlatten();
-  const int length = s->length();
+  const int length = static_cast<int>(Min<uint32_t>(s->length(), limit));

   Handle<FixedArray> elements;
   if (s->IsFlat() && s->IsAsciiRepresentation()) {
Index: src/runtime.h
diff --git a/src/runtime.h b/src/runtime.h
index 756099b413fe6268dcd69a157165d223f841a372..76b3c778ae6f583861273d2209dbeabd70e0b876 100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -175,7 +175,7 @@ namespace internal {
   F(StringReplaceRegExpWithString, 4, 1) \
   F(StringMatch, 3, 1) \
   F(StringTrim, 3, 1) \
-  F(StringToArray, 1, 1) \
+  F(StringToArray, 2, 1) \
   F(NewStringWrapper, 1, 1) \
   \
   /* Numbers */ \
Index: src/string.js
diff --git a/src/string.js b/src/string.js
index d82ce05237ef2def6e2050d9520098db50edeb54..a82a3545962e75c7bd0c7dd32574f7e2eba9b676 100644
--- a/src/string.js
+++ b/src/string.js
@@ -552,7 +552,7 @@ function StringSplit(separator, limit) {
     var separator_length = separator.length;

// If the separator string is empty then return the elements in the subject.
-    if (separator_length === 0) return %StringToArray(subject);
+      if (separator_length === 0) return %StringToArray(subject, limit);

     var result = %StringSplit(subject, separator, limit);

Index: test/mjsunit/string-split.js
diff --git a/test/mjsunit/string-split.js b/test/mjsunit/string-split.js
index c741f6a3e59d639ba4403860927e762890da66d5..6fcf55799e5b3f3b876c972414fd8052caf0fa67 100644
--- a/test/mjsunit/string-split.js
+++ b/test/mjsunit/string-split.js
@@ -97,3 +97,22 @@ assertEquals([""], ''.split(/./));
 assertEquals([], ''.split(/.?/));
 assertEquals([], ''.split(/.??/));
 assertEquals([], ''.split(/()()/));
+
+
+// Issue http://code.google.com/p/v8/issues/detail?id=929
+// (Splitting with empty separator and a limit.)
+
+function numberObj(num) {
+  return {valueOf: function() { return num; }};
+}
+
+assertEquals([], "abc".split("", 0));
+assertEquals([], "abc".split("", numberObj(0)));
+assertEquals(["a"], "abc".split("", 1));
+assertEquals(["a"], "abc".split("", numberObj(1)));
+assertEquals(["a", "b"], "abc".split("", 2));
+assertEquals(["a", "b"], "abc".split("", numberObj(2)));
+assertEquals(["a", "b", "c"], "abc".split("", 3));
+assertEquals(["a", "b", "c"], "abc".split("", numberObj(3)));
+assertEquals(["a", "b", "c"], "abc".split("", 4));
+assertEquals(["a", "b", "c"], "abc".split("", numberObj(4)));


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

Reply via email to