Allow externalization of canonical strings also. Only restrict canonical strings

vm/object.cc

Index: vm/object.cc
===================================================================
--- vm/object.cc	(revision 17180)
+++ vm/object.cc	(working copy)
@@ -2265,7 +2265,7 @@
 
 RawFunction* Class::LookupFunction(const String& name) const {
   Isolate* isolate = Isolate::Current();
-  ASSERT(name.IsOneByteString());
+  ASSERT(name.IsOneByteString() || name.IsExternalOneByteString());

[Ivan Posva, 2013/01/17 07:19:39]

I do not understand why we are limiting this to one byte strings here. There is
no code here that expects this symbol to be a one byte string.

Here and many other places.

I am pretty certain that we can end up in these functions with two byte strings
when coming in through the C API.

   Array& funcs = Array::Handle(isolate, functions());
   if (funcs.IsNull()) {
     // This can occur, e.g., for Null classes.
@@ -2299,7 +2299,7 @@
 
 RawFunction* Class::LookupFunctionAllowPrivate(const String& name) const {
   Isolate* isolate = Isolate::Current();
-  ASSERT(name.IsOneByteString());
+  ASSERT(name.IsOneByteString() || name.IsExternalOneByteString());
   Array& funcs = Array::Handle(isolate, functions());
   if (funcs.IsNull()) {
     // This can occur, e.g., for Null classes.
@@ -2311,8 +2311,8 @@
   for (intptr_t i = 0; i < len; i++) {
     function ^= funcs.At(i);
     function_name ^= function.name();
-    if (OneByteString::EqualsIgnoringPrivateKey(function_name, name)) {
-      return function.raw();
+    if (String::EqualsIgnoringPrivateKey(function_name, name)) {
+        return function.raw();
     }
   }
   // No function found.
@@ -2405,7 +2405,7 @@
 
 RawField* Class::LookupField(const String& name) const {
   Isolate* isolate = Isolate::Current();
-  ASSERT(name.IsOneByteString());
+  ASSERT(name.IsOneByteString() || name.IsExternalOneByteString());
   const Array& flds = Array::Handle(isolate, fields());
   Field& field = Field::Handle(isolate, Field::null());
   String& field_name = String::Handle(isolate, String::null());
@@ -2413,8 +2413,8 @@
   for (intptr_t i = 0; i < len; i++) {
     field ^= flds.At(i);
     field_name ^= field.name();
-    if (OneByteString::EqualsIgnoringPrivateKey(field_name, name)) {
-      return field.raw();
+    if (String::EqualsIgnoringPrivateKey(field_name, name)) {
+        return field.raw();
     }
   }
   // No field found.
@@ -10860,7 +10860,7 @@
   intptr_t original_size = 0;
   uword tags = raw_ptr()->tags_;
 
-  ASSERT(!IsCanonical());
+  ASSERT(!InVMHeap());
   if (class_id == kOneByteStringCid) {
     used_size = ExternalOneByteString::InstanceSize();
     original_size = OneByteString::InstanceSize(str_length);
@@ -10957,6 +10957,85 @@
 }
 
 
+// Check to see if 'str1' matches 'str2' as is or
+// once the private key separator is stripped from str2.
+//
+// Things are made more complicated by the fact that constructors are
+// added *after* the private suffix, so "foo@123.named" should match
+// "foo.named".
+//
+// Also, the private suffix can occur more than once in the name, as in:
+//
+//    _ReceivePortImpl@6be832b._internal@6be832b
+//
+template<typename T1, typename T2>
+static bool EqualsIgnoringPrivateKey(const String& str1,
+                                     const String& str2) {
+  NoGCScope no_gc;
+  intptr_t len = str1.Length();
+  intptr_t str2_len = str2.Length();
+  if (len == str2_len) {
+    for (intptr_t i = 0; i < len; i++) {
+      if (T1::CharAt(str1, i) != T2::CharAt(str2, i)) {
+        return false;
+      }
+    }
+    return true;
+  }
+  if (len < str2_len) {
+    return false;  // No way they can match.
+  }
+  intptr_t pos = 0;
+  intptr_t str2_pos = 0;
+  while (pos < len) {
+    int32_t ch = T1::CharAt(str1, pos);
+    pos++;
+
+    if (ch == Scanner::kPrivateKeySeparator) {
+      // Consume a private key separator.
+      while ((pos < len) && (T1::CharAt(str1, pos) != '.')) {
+        pos++;
+      }
+      // Resume matching characters.
+      continue;
+    }
+    if ((str2_pos == str2_len) || (ch != T2::CharAt(str2, str2_pos))) {
+      return false;
+    }
+    str2_pos++;
+  }
+
+  // We have reached the end of mangled_name string.
+  ASSERT(pos == len);
+  return (str2_pos == str2_len);
+}
+
+
+bool String::EqualsIgnoringPrivateKey(const String& str1,
+                                      const String& str2) {
+  if (str1.raw() == str2.raw()) {
+    return true;  // Both handles point to the same raw instance.
+  }
+  if (str1.IsOneByteString()) {
+    if (str2.IsOneByteString()) {
+      return dart::EqualsIgnoringPrivateKey<OneByteString,
+                                            OneByteString>(str1, str2);
+    }
+    ASSERT(str2.IsExternalOneByteString());
+    return dart::EqualsIgnoringPrivateKey<OneByteString,
+                                          ExternalOneByteString>(str1, str2);
+  }
+  ASSERT(str1.IsExternalOneByteString());
+  if (str2.IsOneByteString()) {
+    return dart::EqualsIgnoringPrivateKey<ExternalOneByteString,
+                                          OneByteString>(str1, str2);
+  }
+  ASSERT(str2.IsExternalOneByteString());
+  return dart::EqualsIgnoringPrivateKey<ExternalOneByteString,
+                                        ExternalOneByteString>(str1, str2);
+}
+
+
 bool String::CodePointIterator::Next() {
   ASSERT(index_ >= -1);
   intptr_t length = Utf16::Length(ch_);
@@ -11010,63 +11089,6 @@
 }
 
 
-// Check to see if 'str1' matches 'str2' as is or
-// once the private key separator is stripped from str2.
-//
-// Things are made more complicated by the fact that constructors are
-// added *after* the private suffix, so "foo@123.named" should match
-// "foo.named".
-//
-// Also, the private suffix can occur more than once in the name, as in:
-//
-//    _ReceivePortImpl@6be832b._internal@6be832b
-//
-bool OneByteString::EqualsIgnoringPrivateKey(const String& str1,
-                                             const String& str2) {
-  ASSERT(str2.IsOneByteString());
-  if (str1.raw() == str2.raw()) {
-    return true;  // Both handles point to the same raw instance.
-  }
-  NoGCScope no_gc;
-  intptr_t len = str1.Length();
-  intptr_t str2_len = str2.Length();
-  if (len == str2_len) {
-    for (intptr_t i = 0; i < len; i++) {
-      if (*CharAddr(str1, i) != *CharAddr(str2, i)) {
-        return false;
-      }
-    }
-    return true;
-  }
-  if (len < str2_len) {
-    return false;  // No way they can match.
-  }
-  intptr_t pos = 0;
-  intptr_t str2_pos = 0;
-  while (pos < len) {
-    int32_t ch = *CharAddr(str1, pos);
-    pos++;
-
-    if (ch == Scanner::kPrivateKeySeparator) {
-      // Consume a private key separator.
-      while ((pos < len) && (*CharAddr(str1, pos) != '.')) {
-        pos++;
-      }
-      // Resume matching characters.
-      continue;
-    }
-    if ((str2_pos == str2_len) || (ch != *CharAddr(str2, str2_pos))) {
-      return false;
-    }
-    str2_pos++;
-  }
-
-  // We have reached the end of mangled_name string.
-  ASSERT(pos == len);
-  return (str2_pos == str2_len);
-}
-
-
 RawOneByteString* OneByteString::New(intptr_t len,
                                      Heap::Space space) {
   ASSERT(Isolate::Current() == Dart::vm_isolate() ||