Reviewers: Erik Corry,

Message:
Please take a look. Fixes some edge cases for ascii-ness checking.

Description:
Changes around ascii-check for strings wrt external strings.


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

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/extensions/externalize-string-extension.cc
  M src/objects-inl.h
  M test/mjsunit/string-slices.js


Index: src/extensions/externalize-string-extension.cc
diff --git a/src/extensions/externalize-string-extension.cc b/src/extensions/externalize-string-extension.cc index 9fbf3298189acdace28f175af3f855b5ae13bd68..d98c586b8f53c8f8ca1570ee20b3bec68bdafa03 100644
--- a/src/extensions/externalize-string-extension.cc
+++ b/src/extensions/externalize-string-extension.cc
@@ -127,8 +127,8 @@ v8::Handle<v8::Value> ExternalizeStringExtension::IsAscii(
     return v8::ThrowException(v8::String::New(
         "isAsciiString() requires a single string argument."));
   }
- return Utils::OpenHandle(*args[0].As<v8::String>())->IsAsciiRepresentation() ?
-      v8::True() : v8::False();
+  Handle<String> string = Utils::OpenHandle(*args[0].As<v8::String>());
+ return string->IsAsciiRepresentationUnderneath() ? v8::True() : v8::False();
 }


Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 267880d7b88994db40c06c02b69787e09e49577a..06f02625569e2c23e9d7bb00a9992685ad59db78 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -290,8 +290,10 @@ bool String::IsAsciiRepresentationUnderneath() {
       return true;
     case kTwoByteStringTag:
       return false;
-    default:  // Cons or sliced string.  Need to go deeper.
-      return GetUnderlying()->IsAsciiRepresentation();
+    default:
+      // Cons or sliced string.  Need to go deeper.
+      // Consider slice of cons of externalized.
+      return GetUnderlying()->IsAsciiRepresentationUnderneath();
   }
 }

Index: test/mjsunit/string-slices.js
diff --git a/test/mjsunit/string-slices.js b/test/mjsunit/string-slices.js
index 7c40229c75def951ac4379af31bf2754ae0bd5c3..2ef10c4efc08ac72134a3cd6c48f9e11f9fb0080 100755
--- a/test/mjsunit/string-slices.js
+++ b/test/mjsunit/string-slices.js
@@ -190,15 +190,33 @@ assertEquals("\u03B2\u03B3\u03B4\u03B5\u03B4\u03B5\u03B6\u03B7",
     utf.substring(5,1) + utf.substring(3,7));

 // Externalizing strings.
-var a = "123456789" + "qwertyuiopasdfghjklzxcvbnm";
-var b = "23456789qwertyuiopasdfghjklzxcvbn"
-assertEquals(a.slice(1,-1), b);
-
-assertTrue(isAsciiString(a));
-externalizeString(a, true);
-assertFalse(isAsciiString(a));
-
-assertEquals(a.slice(1,-1), b);
-assertTrue(/3456789qwe/.test(a));
-assertEquals(5, a.indexOf("678"));
-assertEquals("12345", a.split("6")[0]);
+seq = "123456789qwertyuiopasdfghjklzxcvbnm";
+cons = "123456789" + "qwertyuiopasdfghjklzxcvbnm";
+check = "23456789qwertyuiopasdfghjklzxcvbn";
+
+// Externalizing a sequential string changes its encoding from ascii to
+// two-byte. The encoding of the sliced string must reflect this change too.
+slice = seq.slice(1,-1);
+assertEquals(check, slice);
+
+// Seq strings can only be externalized once across multiple stress-opt runs.
+if (isAsciiString(seq)) externalizeString(seq, true);
+assertFalse(isAsciiString(seq));
+assertFalse(isAsciiString(slice));
+
+assertEquals(check, seq.slice(1,-1));
+assertEquals(check, slice);
+assertTrue(/3456789qwe/.test(seq));
+assertEquals(5, seq.indexOf("678"));
+assertEquals("12345", seq.split("6")[0]);
+
+// Externalizing a cons string changes its encoding from ascii to two-byte,
+// but the slice depending on the cons string does not, as it still points to
+// the original cons string.
+slice = cons.slice(1,-1);
+assertEquals(check, slice);
+
+assertTrue(isAsciiString(cons));
+externalizeString(cons, true);
+assertFalse(isAsciiString(cons));
+assertTrue(isAsciiString(slice));


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

Reply via email to