Merged String subclasses into String.

runtime/vm/raw_object_snapshot.cc

Index: runtime/vm/raw_object_snapshot.cc
diff --git a/runtime/vm/raw_object_snapshot.cc b/runtime/vm/raw_object_snapshot.cc
index d817ff1e3d706e55c34366432926a8d2141fab46..698f1d02c6a3b97784b5c238b2a3dd53191ec668 100644
--- a/runtime/vm/raw_object_snapshot.cc
+++ b/runtime/vm/raw_object_snapshot.cc
@@ -1628,7 +1628,7 @@ void RawString::WriteTo(SnapshotWriter* writer,
 
 template<typename HandleType, typename CharacterType>
 void String::ReadFromImpl(SnapshotReader* reader,
-                          HandleType* str_obj,
+                          String* str_obj,
                           intptr_t len,
                           intptr_t tags,
                           Snapshot::Kind kind) {
@@ -1647,8 +1647,10 @@ void String::ReadFromImpl(SnapshotReader* reader,
     *str_obj = HandleType::New(len, HEAP_SPACE(kind));
     str_obj->set_tags(tags);
     str_obj->SetHash(0);  // Will get computed when needed.
+    CharacterType* dst_chars =
+        const_cast<CharacterType*>(HandleType::CharAddr(*str_obj, 0));
     for (intptr_t i = 0; i < len; i++) {
-      *str_obj->CharAddr(i) = reader->Read<CharacterType>();
+      dst_chars[i] = reader->Read<CharacterType>();

[siva, 2012/11/02 01:15:25]

dst_chars is pointing to a raw pointer and we are looping through without any
NOGCScope here.

why not leave the loop as is, that should work right?  

[Tom Ball, 2012/11/02 23:31:50]

Restored original loop, updated it.

     }
   }
 }
@@ -1662,8 +1664,8 @@ RawOneByteString* OneByteString::ReadFrom(SnapshotReader* reader,
   ASSERT(reader != NULL);
   intptr_t len = reader->ReadSmiValue();
   intptr_t hash = reader->ReadSmiValue();
-  OneByteString& str_obj = OneByteString::ZoneHandle(reader->isolate(),
-                                                     OneByteString::null());
+  String& str_obj = OneByteString::ZoneHandle(reader->isolate(),

[siva, 2012/11/02 01:15:25]

Should be String::ZoneHandle(.....);

[Tom Ball, 2012/11/02 23:31:50]

Done.

+                                              String::null());
 
   if (kind == Snapshot::kFull) {
     ASSERT(reader->isolate()->no_gc_scope_depth() != 0);
@@ -1672,7 +1674,8 @@ RawOneByteString* OneByteString::ReadFrom(SnapshotReader* reader,
     str_obj.set_tags(tags);
     obj->ptr()->hash_ = Smi::New(hash);
     if (len > 0) {
-      uint8_t* raw_ptr = str_obj.CharAddr(0);
+      uint8_t* raw_ptr =
+          const_cast<uint8_t*>(OneByteString::CharAddr(str_obj, 0));

[siva, 2012/11/02 01:15:25]

why is this const_cast needed?

[Tom Ball, 2012/11/02 23:31:50]

Removed.

       reader->ReadBytes(raw_ptr, len);
     }
     ASSERT((hash == 0) || (String::Hash(str_obj, 0, str_obj.Length()) == hash));
@@ -1680,7 +1683,7 @@ RawOneByteString* OneByteString::ReadFrom(SnapshotReader* reader,
     ReadFromImpl<OneByteString, uint8_t>(reader, &str_obj, len, tags, kind);
   }
   reader->AddBackRef(object_id, &str_obj, kIsDeserialized);
-  return str_obj.raw();
+  return reinterpret_cast<RawOneByteString*>(str_obj.raw());

[siva, 2012/11/02 01:15:25]

I am not sure how many places you are having to do this
kind of cast , it might make sense to have a
OneByteString::raw(str_obj);
which does the case inside and returns.

[Tom Ball, 2012/11/02 23:31:50]

Good idea -- done.

 }
 
 
@@ -1692,17 +1695,19 @@ RawTwoByteString* TwoByteString::ReadFrom(SnapshotReader* reader,
   ASSERT(reader != NULL);
   intptr_t len = reader->ReadSmiValue();
   intptr_t hash = reader->ReadSmiValue();
-  TwoByteString& str_obj = TwoByteString::ZoneHandle(reader->isolate(),
-                                                     TwoByteString::null());
+  String& str_obj = TwoByteString::ZoneHandle(reader->isolate(),

[siva, 2012/11/02 01:15:25]

Should be String::ZoneHandle(...);

[Tom Ball, 2012/11/02 23:31:50]

Done.

+                                              String::null());
 
   if (kind == Snapshot::kFull) {
     RawTwoByteString* obj = reader->NewTwoByteString(len);
     str_obj = obj;
     str_obj.set_tags(tags);
     obj->ptr()->hash_ = Smi::New(hash);
-    uint16_t* raw_ptr = (len > 0)? str_obj.CharAddr(0) : NULL;
+    uint16_t* raw_ptr = (len > 0)?
+        const_cast<uint16_t*>(TwoByteString::CharAddr(str_obj, 0)) : NULL;

[siva, 2012/11/02 01:15:25]

Why is this const_cast needed?

[Tom Ball, 2012/11/02 23:31:50]

Removed.

     for (intptr_t i = 0; i < len; i++) {
-      ASSERT(str_obj.CharAddr(i) == raw_ptr);  // Will trigger assertions.
+      // Will trigger assertions.
+      ASSERT(TwoByteString::CharAddr(str_obj, i) == raw_ptr);
       *raw_ptr = reader->Read<uint16_t>();
       raw_ptr += 1;
     }
@@ -1711,7 +1716,7 @@ RawTwoByteString* TwoByteString::ReadFrom(SnapshotReader* reader,
     ReadFromImpl<TwoByteString, uint16_t>(reader, &str_obj, len, tags, kind);
   }
   reader->AddBackRef(object_id, &str_obj, kIsDeserialized);
-  return str_obj.raw();
+  return reinterpret_cast<RawTwoByteString*>(str_obj.raw());

[siva, 2012/11/02 01:15:25]

Ditto comment for TwoByteString::raw(str_obj);

[Tom Ball, 2012/11/02 23:31:50]

Done.

 }