Remove all uses of StringShape variables, since that has proven

to be error-prone and of little benefit in terms of performance.
Review URL: http://codereview.chromium.org/45010

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1521 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -2034,7 +2034,7 @@ int String::WriteAscii(char* buffer, int start, int length) const {
   i::Handle<i::String> str = Utils::OpenHandle(this);
   // Flatten the string for efficiency.  This applies whether we are
   // using StringInputBuffer or Get(i) to access the characters.
-  str->TryFlattenIfNotFlat(i::StringShape(*str));
+  str->TryFlattenIfNotFlat();
   int end = length;
   if ( (length == -1) || (length > str->length() - start) )
     end = str->length() - start;
@@ -2059,7 +2059,7 @@ int String::Write(uint16_t* buffer, int start, int length) const {
   i::Handle<i::String> str = Utils::OpenHandle(this);
   // Flatten the string for efficiency.  This applies whether we are
   // using StringInputBuffer or Get(i) to access the characters.
-  str->TryFlattenIfNotFlat(i::StringShape(*str));
+  str->TryFlattenIfNotFlat();
   int end = length;
   if ( (length == -1) || (length > str->length() - start) )
     end = str->length() - start;
@@ -2077,16 +2077,14 @@ int String::Write(uint16_t* buffer, int start, int length) const {
 bool v8::String::IsExternal() const {
   EnsureInitialized("v8::String::IsExternal()");
   i::Handle<i::String> str = Utils::OpenHandle(this);
-  i::StringShape shape(*str);
-  return shape.IsExternalTwoByte();
+  return i::StringShape(*str).IsExternalTwoByte();
 }
 
 
 bool v8::String::IsExternalAscii() const {
   EnsureInitialized("v8::String::IsExternalAscii()");
   i::Handle<i::String> str = Utils::OpenHandle(this);
-  i::StringShape shape(*str);
-  return shape.IsExternalAscii();
+  return i::StringShape(*str).IsExternalAscii();
 }
 
 

src/codegen.cc

@@ -402,8 +402,7 @@ bool CodeGenerator::CheckForInlineRuntimeCall(CallRuntime* node) {
      "_Log"}
   };
   Handle<String> name = node->name();
-  StringShape shape(*name);
-  if (name->length(shape) > 0 && name->Get(shape, 0) == '_') {
+  if (name->length() > 0 && name->Get(0) == '_') {
     for (unsigned i = 0;
          i < sizeof(kInlineRuntimeLUT) / sizeof(InlineRuntimeLUT);
          i++) {

src/conversions.cc

@@ -55,8 +55,7 @@ static inline int GetChar(const char* str, int index) {
 
 
 static inline int GetChar(String* str, int index) {
-  StringShape shape(str);
-  return str->Get(shape, index);
+  return str->Get(index);
 }
 
 
@@ -76,11 +75,10 @@ static inline const char* GetCString(const char* str, int index) {
 
 
 static inline const char* GetCString(String* str, int index) {
-  StringShape shape(str);
-  int length = str->length(shape);
+  int length = str->length();
   char* result = NewArray<char>(length + 1);
   for (int i = index; i < length; i++) {
-    uc16 c = str->Get(shape, i);
+    uc16 c = str->Get(i);
     if (c <= 127) {
       result[i - index] = static_cast<char>(c);
     } else {
@@ -108,8 +106,7 @@ static inline bool IsSpace(const char* str, int index) {
 
 
 static inline bool IsSpace(String* str, int index) {
-  StringShape shape(str);
-  return Scanner::kIsWhiteSpace.get(str->Get(shape, index));
+  return Scanner::kIsWhiteSpace.get(str->Get(index));
 }
 
 

src/factory.cc

@@ -90,11 +90,9 @@ Handle<String> Factory::NewRawTwoByteString(int length,
 
 
 Handle<String> Factory::NewConsString(Handle<String> first,
-                                      StringShape first_shape,
-                                      Handle<String> second,
-                                      StringShape second_shape) {
-  if (first->length(first_shape) == 0) return second;
-  if (second->length(second_shape) == 0) return first;
+                                      Handle<String> second) {
+  if (first->length() == 0) return second;
+  if (second->length() == 0) return first;
   CALL_HEAP_FUNCTION(Heap::AllocateConsString(*first, *second), String);
 }
 

src/factory.h

@@ -98,9 +98,7 @@ class Factory : public AllStatic {
 
   // Create a new cons string object which consists of a pair of strings.
   static Handle<String> NewConsString(Handle<String> first,
-                                      StringShape first_shape,
-                                      Handle<String> second,
-                                      StringShape second_shape);
+                                      Handle<String> second);
 
   // Create a new sliced string object which represents a substring of a
   // backing string.

src/handles.cc

@@ -186,8 +186,8 @@ void TransformToFastProperties(Handle<JSObject> object,
 
 
 void FlattenString(Handle<String> string) {
-  CALL_HEAP_FUNCTION_VOID(string->TryFlattenIfNotFlat(StringShape(*string)));
-  ASSERT(string->IsFlat(StringShape(*string)));
+  CALL_HEAP_FUNCTION_VOID(string->TryFlattenIfNotFlat());
+  ASSERT(string->IsFlat());
 }
 
 

src/heap.cc

@@ -230,7 +230,6 @@ void Heap::ReportStatisticsAfterGC() {
 
 
 void Heap::GarbageCollectionPrologue() {
-  RegExpImpl::NewSpaceCollectionPrologue();
   gc_count_++;
 #ifdef DEBUG
   ASSERT(allocation_allowed_ && gc_state_ == NOT_IN_GC);
@@ -475,7 +474,6 @@ void Heap::MarkCompactPrologue(bool is_compacting) {
   ClearKeyedLookupCache();
 
   CompilationCache::MarkCompactPrologue();
-  RegExpImpl::OldSpaceCollectionPrologue();
 
   Top::MarkCompactPrologue(is_compacting);
   ThreadManager::MarkCompactPrologue(is_compacting);
@@ -1392,41 +1390,31 @@ Object* Heap::AllocateSharedFunctionInfo(Object* name) {
 
 Object* Heap::AllocateConsString(String* first,
                                  String* second) {
-  StringShape first_shape(first);
-  StringShape second_shape(second);
-  int first_length = first->length(first_shape);
-  int second_length = second->length(second_shape);
+  int first_length = first->length();
+  int second_length = second->length();
   int length = first_length + second_length;
-  bool is_ascii = first_shape.IsAsciiRepresentation()
-      && second_shape.IsAsciiRepresentation();
+  bool is_ascii = StringShape(first).IsAsciiRepresentation()
+      && StringShape(second).IsAsciiRepresentation();
 
   // If the resulting string is small make a flat string.
   if (length < String::kMinNonFlatLength) {
-    ASSERT(first->IsFlat(first_shape));
-    ASSERT(second->IsFlat(second_shape));
+    ASSERT(first->IsFlat());
+    ASSERT(second->IsFlat());
     if (is_ascii) {
       Object* result = AllocateRawAsciiString(length);
       if (result->IsFailure()) return result;
       // Copy the characters into the new object.
       char* dest = SeqAsciiString::cast(result)->GetChars();
-      String::WriteToFlat(first, first_shape, dest, 0, first_length);
-      String::WriteToFlat(second,
-                          second_shape,
-                          dest + first_length,
-                          0,
-                          second_length);
+      String::WriteToFlat(first, dest, 0, first_length);
+      String::WriteToFlat(second, dest + first_length, 0, second_length);
       return result;
     } else {
       Object* result = AllocateRawTwoByteString(length);
       if (result->IsFailure()) return result;
       // Copy the characters into the new object.
       uc16* dest = SeqTwoByteString::cast(result)->GetChars();
-      String::WriteToFlat(first, first_shape, dest, 0, first_length);
-      String::WriteToFlat(second,
-                          second_shape,
-                          dest + first_length,
-                          0,
-                          second_length);
+      String::WriteToFlat(first, dest, 0, first_length);
+      String::WriteToFlat(second, dest + first_length, 0, second_length);
       return result;
     }
   }
@@ -1457,25 +1445,24 @@ Object* Heap::AllocateConsString(String* first,
 Object* Heap::AllocateSlicedString(String* buffer,
                                    int start,
                                    int end) {
-  StringShape buffer_shape(buffer);
   int length = end - start;
 
   // If the resulting string is small make a sub string.
   if (end - start <= String::kMinNonFlatLength) {
-    return Heap::AllocateSubString(buffer, buffer_shape, start, end);
+    return Heap::AllocateSubString(buffer, start, end);
   }
 
   Map* map;
   if (length <= String::kMaxShortStringSize) {
-    map = buffer_shape.IsAsciiRepresentation() ?
+    map = StringShape(buffer).IsAsciiRepresentation() ?
       short_sliced_ascii_string_map() :
       short_sliced_string_map();
   } else if (length <= String::kMaxMediumStringSize) {
-    map = buffer_shape.IsAsciiRepresentation() ?
+    map = StringShape(buffer).IsAsciiRepresentation() ?
       medium_sliced_ascii_string_map() :
       medium_sliced_string_map();
   } else {
-    map = buffer_shape.IsAsciiRepresentation() ?
+    map = StringShape(buffer).IsAsciiRepresentation() ?
       long_sliced_ascii_string_map() :
       long_sliced_string_map();
   }
@@ -1493,41 +1480,38 @@ Object* Heap::AllocateSlicedString(String* buffer,
 
 
 Object* Heap::AllocateSubString(String* buffer,
-                                StringShape buffer_shape,
                                 int start,
                                 int end) {
   int length = end - start;
 
   if (length == 1) {
     return Heap::LookupSingleCharacterStringFromCode(
-        buffer->Get(buffer_shape, start));
+        buffer->Get(start));
   }
 
   // Make an attempt to flatten the buffer to reduce access time.
-  if (!buffer->IsFlat(buffer_shape)) {
-    buffer->TryFlatten(buffer_shape);
-    buffer_shape = StringShape(buffer);
+  if (!buffer->IsFlat()) {
+    buffer->TryFlatten();
   }
 
-  Object* result = buffer_shape.IsAsciiRepresentation()
+  Object* result = StringShape(buffer).IsAsciiRepresentation()
       ? AllocateRawAsciiString(length)
       : AllocateRawTwoByteString(length);
   if (result->IsFailure()) return result;
 
   // Copy the characters into the new object.
   String* string_result = String::cast(result);
-  StringShape result_shape(string_result);
   StringHasher hasher(length);
   int i = 0;
   for (; i < length && hasher.is_array_index(); i++) {
-    uc32 c = buffer->Get(buffer_shape, start + i);
+    uc32 c = buffer->Get(start + i);
     hasher.AddCharacter(c);
-    string_result->Set(result_shape, i, c);
+    string_result->Set(i, c);
   }
   for (; i < length; i++) {
-    uc32 c = buffer->Get(buffer_shape, start + i);
+    uc32 c = buffer->Get(start + i);
     hasher.AddCharacterNoIndex(c);
-    string_result->Set(result_shape, i, c);
+    string_result->Set(i, c);
   }
   string_result->set_length_field(hasher.GetHashField());
   return result;
@@ -1590,7 +1574,7 @@ Object* Heap::LookupSingleCharacterStringFromCode(uint16_t code) {
   Object* result = Heap::AllocateRawTwoByteString(1);
   if (result->IsFailure()) return result;
   String* answer = String::cast(result);
-  answer->Set(StringShape(answer), 0, code);
+  answer->Set(0, code);
   return answer;
 }
 
@@ -2016,10 +2000,9 @@ Object* Heap::AllocateStringFromUtf8(Vector<const char> string,
   // Convert and copy the characters into the new object.
   String* string_result = String::cast(result);
   decoder->Reset(string.start(), string.length());
-  StringShape result_shape(string_result);
   for (int i = 0; i < chars; i++) {
     uc32 r = decoder->GetNext();
-    string_result->Set(result_shape, i, r);
+    string_result->Set(i, r);
   }
   return result;
 }
@@ -2042,9 +2025,8 @@ Object* Heap::AllocateStringFromTwoByte(Vector<const uc16> string,
   // Copy the characters into the new object, which may be either ASCII or
   // UTF-16.
   String* string_result = String::cast(result);
-  StringShape result_shape(string_result);
   for (int i = 0; i < string.length(); i++) {
-    string_result->Set(result_shape, i, string[i]);
+    string_result->Set(i, string[i]);
   }
   return result;
 }
@@ -2163,14 +2145,13 @@ Object* Heap::AllocateInternalSymbol(unibrow::CharacterStream* buffer,
   reinterpret_cast<HeapObject*>(result)->set_map(map);
   // The hash value contains the length of the string.
   String* answer = String::cast(result);
-  StringShape answer_shape(answer);
   answer->set_length_field(length_field);
 
   ASSERT_EQ(size, answer->Size());
 
   // Fill in the characters.
   for (int i = 0; i < chars; i++) {
-    answer->Set(answer_shape, i, buffer->GetNext());
+    answer->Set(i, buffer->GetNext());
   }
   return answer;
 }

src/heap.h

@@ -532,7 +532,6 @@ class Heap : public AllStatic {
   // failed.
   // Please note this does not perform a garbage collection.
   static Object* AllocateSubString(String* buffer,
-                                   StringShape buffer_shape,
                                    int start,
                                    int end);
 

src/interpreter-irregexp.cc

@@ -569,13 +569,12 @@ bool IrregexpInterpreter::Match(Handle<ByteArray> code_array,
                                 Handle<String> subject,
                                 int* registers,
                                 int start_position) {
-  ASSERT(subject->IsFlat(StringShape(*subject)));
+  ASSERT(subject->IsFlat());
 
   AssertNoAllocation a;
   const byte* code_base = code_array->GetDataStartAddress();
-  StringShape subject_shape(*subject);
   uc16 previous_char = '\n';
-  if (subject_shape.IsAsciiRepresentation()) {
+  if (StringShape(*subject).IsAsciiRepresentation()) {
     Vector<const char> subject_vector = subject->ToAsciiVector();
     if (start_position != 0) previous_char = subject_vector[start_position - 1];
     return RawMatch(code_base,

src/jsregexp.cc

@@ -55,27 +55,6 @@
 namespace v8 { namespace internal {
 
 
-String* RegExpImpl::last_ascii_string_ = NULL;
-String* RegExpImpl::two_byte_cached_string_ = NULL;
-
-
-void RegExpImpl::NewSpaceCollectionPrologue() {
-  // The two byte string is always in the old space.  The Ascii string may be
-  // in either place.  If it is in the old space we don't need to do anything.
-  if (Heap::InNewSpace(last_ascii_string_)) {
-    // Invalidate the cache.
-    last_ascii_string_ = NULL;
-    two_byte_cached_string_ = NULL;
-  }
-}
-
-
-void RegExpImpl::OldSpaceCollectionPrologue() {
-  last_ascii_string_ = NULL;
-  two_byte_cached_string_ = NULL;
-}
-
-
 Handle<Object> RegExpImpl::CreateRegExpLiteral(Handle<JSFunction> constructor,
                                                Handle<String> pattern,
                                                Handle<String> flags,
@@ -92,55 +71,10 @@ Handle<Object> RegExpImpl::CreateRegExpLiteral(Handle<JSFunction> constructor,
 }
 
 
-// Converts a source string to a 16 bit flat string or a SlicedString containing
-// a 16 bit flat string).
-Handle<String> RegExpImpl::CachedStringToTwoByte(Handle<String> subject) {
-  if (*subject == last_ascii_string_) {
-    ASSERT(two_byte_cached_string_ != NULL);
-    return Handle<String>(String::cast(two_byte_cached_string_));
-  }
-  Handle<String> two_byte_string = StringToTwoByte(subject);
-  last_ascii_string_ = *subject;
-  two_byte_cached_string_ = *two_byte_string;
-  return two_byte_string;
-}
-
-
-// Converts a source string to a 16 bit flat string or a SlicedString containing
-// a 16 bit flat string).
-Handle<String> RegExpImpl::StringToTwoByte(Handle<String> pattern) {
-  StringShape shape(*pattern);
-  if (!pattern->IsFlat(shape)) {
-    FlattenString(pattern);
-    shape = StringShape(*pattern);
-  }
-  Handle<String> flat_string(shape.IsCons() ?
-    String::cast(ConsString::cast(*pattern)->first()) :
-    *pattern);
-  ASSERT(flat_string->IsString());
-  StringShape flat_shape(*flat_string);
-  ASSERT(!flat_shape.IsCons());
-  ASSERT(flat_shape.IsSequential() ||
-         flat_shape.IsSliced() ||
-         flat_shape.IsExternal());
-  if (!flat_shape.IsAsciiRepresentation()) {
-    return flat_string;
-  }
-
-  int len = flat_string->length(flat_shape);
-  Handle<String> two_byte_string =
-    Factory::NewRawTwoByteString(len, TENURED);
-  uc16* dest = SeqTwoByteString::cast(*two_byte_string)->GetChars();
-  String::WriteToFlat(*flat_string, flat_shape, dest, 0, len);
-  return two_byte_string;
-}
-
-
 static JSRegExp::Flags RegExpFlagsFromString(Handle<String> str) {
   int flags = JSRegExp::NONE;
-  StringShape shape(*str);
-  for (int i = 0; i < str->length(shape); i++) {
-    switch (str->Get(shape, i)) {
+  for (int i = 0; i < str->length(); i++) {
+    switch (str->Get(i)) {
       case 'i':
         flags |= JSRegExp::IGNORE_CASE;
         break;
@@ -421,7 +355,7 @@ bool RegExpImpl::EnsureCompiledIrregexp(Handle<JSRegExp> re,
   JSRegExp::Flags flags = re->GetFlags();
 
   Handle<String> pattern(re->Pattern());
-  if (!pattern->IsFlat(StringShape(*pattern))) {
+  if (!pattern->IsFlat()) {
     FlattenString(pattern);
   }
 
@@ -552,7 +486,7 @@ Handle<Object> RegExpImpl::IrregexpExec(Handle<JSRegExp> regexp,
   }
 #endif
 
-  if (!subject->IsFlat(StringShape(*subject))) {
+  if (!subject->IsFlat()) {
     FlattenString(subject);
   }
 
@@ -590,7 +524,7 @@ Handle<Object> RegExpImpl::IrregexpExecGlobal(Handle<JSRegExp> regexp,
   int result_length = 0;
   Handle<Object> matches;
 
-  if (!subject->IsFlat(StringShape(*subject))) {
+  if (!subject->IsFlat()) {
     FlattenString(subject);
   }
 
@@ -659,9 +593,8 @@ Handle<Object> RegExpImpl::IrregexpExecOnce(Handle<FixedArray> regexp,
                                             int previous_index,
                                             int* offsets_vector,
                                             int offsets_vector_length) {
-  StringShape shape(*subject);
-  ASSERT(subject->IsFlat(shape));
-  bool is_ascii = shape.IsAsciiRepresentation();
+  ASSERT(subject->IsFlat());
+  bool is_ascii = StringShape(*subject).IsAsciiRepresentation();
   bool rc;
 
   Handle<String> original_subject = subject;

src/jsregexp.h

@@ -108,15 +108,6 @@ class RegExpImpl {
                                            Handle<String> subject,
                                            Handle<JSArray> lastMatchInfo);
 
-  static void NewSpaceCollectionPrologue();
-  static void OldSpaceCollectionPrologue();
-
-  // Converts a source string to a 16 bit flat string.  The string
-  // will be either sequential or it will be a SlicedString backed
-  // by a flat string.
-  static Handle<String> StringToTwoByte(Handle<String> pattern);
-  static Handle<String> CachedStringToTwoByte(Handle<String> pattern);
-
   // Offsets in the lastMatchInfo array.
   static const int kLastCaptureCount = 0;
   static const int kLastSubject = 1;

src/log.cc

@@ -362,29 +362,27 @@ void LogMessageBuilder::Append(const char c) {
 // Append a heap string.
 void LogMessageBuilder::Append(String* str) {
   AssertNoAllocation no_heap_allocation;  // Ensure string stay valid.
-  StringShape shape(str);
-  int length = str->length(shape);
+  int length = str->length();
   for (int i = 0; i < length; i++) {
-    Append(static_cast<char>(str->Get(shape, i)));
+    Append(static_cast<char>(str->Get(i)));
   }
 }
 
 void LogMessageBuilder::AppendDetailed(String* str, bool show_impl_info) {
   AssertNoAllocation no_heap_allocation;  // Ensure string stay valid.
-  StringShape shape(str);
-  int len = str->length(shape);
+  int len = str->length();
   if (len > 0x1000)
     len = 0x1000;
   if (show_impl_info) {
-    Append(shape.IsAsciiRepresentation() ? 'a' : '2');
-    if (shape.IsExternal())
+    Append(StringShape(str).IsAsciiRepresentation() ? 'a' : '2');
+    if (StringShape(str).IsExternal())
       Append('e');
-    if (shape.IsSymbol())
+    if (StringShape(str).IsSymbol())
       Append('#');
     Append(":%i:", str->length());
   }
   for (int i = 0; i < len; i++) {
-    uc32 c = str->Get(shape, i);
+    uc32 c = str->Get(i);
     if (c > 0xff) {
       Append("\\u%04x", c);
     } else if (c < 32 || c > 126) {

src/objects-debug.cc

@@ -491,20 +491,19 @@ void JSValue::JSValueVerify() {
 
 
 void String::StringPrint() {
-  StringShape shape(this);
-  if (shape.IsSymbol()) {
+  if (StringShape(this).IsSymbol()) {
     PrintF("#");
-  } else if (shape.IsCons()) {
+  } else if (StringShape(this).IsCons()) {
     PrintF("c\"");
   } else {
     PrintF("\"");
   }
 
   for (int i = 0; i < length(); i++) {
-    PrintF("%c", Get(shape, i));
+    PrintF("%c", Get(i));
   }
 
-  if (!shape.IsSymbol()) PrintF("\"");
+  if (!StringShape(this).IsSymbol()) PrintF("\"");
 }
 
 

src/objects-inl.h

@@ -143,15 +143,15 @@ bool Object::IsSeqString() {
 
 bool Object::IsSeqAsciiString() {
   if (!IsString()) return false;
-  StringShape shape(String::cast(this));
-  return shape.IsSequential() && shape.IsAsciiRepresentation();
+  return StringShape(String::cast(this)).IsSequential() &&
+         StringShape(String::cast(this)).IsAsciiRepresentation();
 }
 
 
 bool Object::IsSeqTwoByteString() {
   if (!IsString()) return false;
-  StringShape shape(String::cast(this));
-  return shape.IsSequential() && shape.IsTwoByteRepresentation();
+  return StringShape(String::cast(this)).IsSequential() &&
+         StringShape(String::cast(this)).IsTwoByteRepresentation();
 }
 
 
@@ -163,15 +163,15 @@ bool Object::IsExternalString() {
 
 bool Object::IsExternalAsciiString() {
   if (!IsString()) return false;
-  StringShape shape(String::cast(this));
-  return shape.IsExternal() && shape.IsAsciiRepresentation();
+  return StringShape(String::cast(this)).IsExternal() &&
+         StringShape(String::cast(this)).IsAsciiRepresentation();
 }
 
 
 bool Object::IsExternalTwoByteString() {
   if (!IsString()) return false;
-  StringShape shape(String::cast(this));
-  return shape.IsExternal() && shape.IsTwoByteRepresentation();
+  return StringShape(String::cast(this)).IsExternal() &&
+         StringShape(String::cast(this)).IsTwoByteRepresentation();
 }
 
 
@@ -1243,15 +1243,13 @@ void DescriptorArray::fast_swap(FixedArray* array, int first, int second) {
 int DescriptorArray::Search(String* name) {
   SLOW_ASSERT(IsSortedNoDuplicates());
 
-  StringShape shape(name);
-
   // Check for empty descriptor array.
   int nof = number_of_descriptors();
   if (nof == 0) return kNotFound;
 
   // Fast case: do linear search for small arrays.
   const int kMaxElementsForLinearSearch = 8;
-  if (shape.IsSymbol() && nof < kMaxElementsForLinearSearch) {
+  if (StringShape(name).IsSymbol() && nof < kMaxElementsForLinearSearch) {
     return LinearSearch(name, nof);
   }
 
@@ -1392,27 +1390,21 @@ INT_ACCESSORS(Array, length, kLengthOffset)
 
 bool String::Equals(String* other) {
   if (other == this) return true;
-  StringShape this_shape(this);
-  StringShape other_shape(other);
-  if (this_shape.IsSymbol() && other_shape.IsSymbol()) return false;
-  return SlowEquals(this_shape, other, other_shape);
+  if (StringShape(this).IsSymbol() && StringShape(other).IsSymbol()) {
+    return false;
+  }
+  return SlowEquals(other);
 }
 
 
-int String::length(StringShape shape) {
-  ASSERT(shape.type() == StringShape(this).type());
+int String::length() {
   uint32_t len = READ_INT_FIELD(this, kLengthOffset);
 
   ASSERT(kShortStringTag + kLongLengthShift == kShortLengthShift);
   ASSERT(kMediumStringTag + kLongLengthShift == kMediumLengthShift);
   ASSERT(kLongStringTag == 0);
 
-  return len >> (shape.size_tag() + kLongLengthShift);
-}
-
-
-int String::length() {
-  return length(StringShape(this));
+  return len >> (StringShape(this).size_tag() + kLongLengthShift);
 }
 
 
@@ -1421,10 +1413,9 @@ void String::set_length(int value) {
   ASSERT(kMediumStringTag + kLongLengthShift == kMediumLengthShift);
   ASSERT(kLongStringTag == 0);
 
-  StringShape shape(this);
   WRITE_INT_FIELD(this,
                   kLengthOffset,
-                  value << (shape.size_tag() + kLongLengthShift));
+                  value << (StringShape(this).size_tag() + kLongLengthShift));
 }
 
 
@@ -1438,21 +1429,19 @@ void String::set_length_field(uint32_t value) {
 }
 
 
-Object* String::TryFlattenIfNotFlat(StringShape shape) {
-  ASSERT(shape.type() == StringShape(this).type());
+Object* String::TryFlattenIfNotFlat() {
   // We don't need to flatten strings that are already flat.  Since this code
   // is inlined, it can be helpful in the flat case to not call out to Flatten.
-  if (!IsFlat(shape)) {
-    return TryFlatten(shape);
+  if (!IsFlat()) {
+    return TryFlatten();
   }
   return this;
 }
 
 
-uint16_t String::Get(StringShape shape, int index) {
-  ASSERT(shape.type() == StringShape(this).type());
-  ASSERT(index >= 0 && index < length(shape));
-  switch (shape.full_representation_tag()) {
+uint16_t String::Get(int index) {
+  ASSERT(index >= 0 && index < length());
+  switch (StringShape(this).full_representation_tag()) {
     case kSeqStringTag | kAsciiStringTag:
       return SeqAsciiString::cast(this)->SeqAsciiStringGet(index);
     case kSeqStringTag | kTwoByteStringTag:
@@ -1476,29 +1465,26 @@ uint16_t String::Get(StringShape shape, int index) {
 }
 
 
-void String::Set(StringShape shape, int index, uint16_t value) {
-  ASSERT(shape.type() == StringShape(this).type());
-  ASSERT(shape.type() == StringShape(this).type());
-  ASSERT(index >= 0 && index < length(shape));
-  ASSERT(shape.IsSequential());
+void String::Set(int index, uint16_t value) {
+  ASSERT(index >= 0 && index < length());
+  ASSERT(StringShape(this).IsSequential());
 
-  return shape.IsAsciiRepresentation()
+  return StringShape(this).IsAsciiRepresentation()
       ? SeqAsciiString::cast(this)->SeqAsciiStringSet(index, value)
       : SeqTwoByteString::cast(this)->SeqTwoByteStringSet(index, value);
 }
 
 
-bool String::IsFlat(StringShape shape) {
-  ASSERT(shape.type() == StringShape(this).type());
-  switch (shape.representation_tag()) {
+bool String::IsFlat() {
+  switch (StringShape(this).representation_tag()) {
     case kConsStringTag: {
       String* second = ConsString::cast(this)->second();
       // Only flattened strings have second part empty.
       return second->length() == 0;
     }
     case kSlicedStringTag: {
-      StringShape slice_shape = StringShape(SlicedString::cast(this)->buffer());
-      StringRepresentationTag tag = slice_shape.representation_tag();
+      StringRepresentationTag tag =
+          StringShape(SlicedString::cast(this)->buffer()).representation_tag();
       return tag == kSeqStringTag || tag == kExternalStringTag;
     }
     default:
@@ -1552,7 +1538,7 @@ void SeqTwoByteString::SeqTwoByteStringSet(int index, uint16_t value) {
 }
 
 
-int SeqTwoByteString::SeqTwoByteStringSize(StringShape shape) {
+int SeqTwoByteString::SeqTwoByteStringSize(InstanceType instance_type) {
   uint32_t length = READ_INT_FIELD(this, kLengthOffset);
 
   ASSERT(kShortStringTag + kLongLengthShift == kShortLengthShift);
@@ -1561,13 +1547,13 @@ int SeqTwoByteString::SeqTwoByteStringSize(StringShape shape) {
 
   // Use the map (and not 'this') to compute the size tag, since
   // TwoByteStringSize is called during GC when maps are encoded.
-  length >>= shape.size_tag() + kLongLengthShift;
+  length >>= StringShape(instance_type).size_tag() + kLongLengthShift;
 
   return SizeFor(length);
 }
 
 
-int SeqAsciiString::SeqAsciiStringSize(StringShape shape) {
+int SeqAsciiString::SeqAsciiStringSize(InstanceType instance_type) {
   uint32_t length = READ_INT_FIELD(this, kLengthOffset);
 
   ASSERT(kShortStringTag + kLongLengthShift == kShortLengthShift);
@@ -1576,7 +1562,7 @@ int SeqAsciiString::SeqAsciiStringSize(StringShape shape) {
 
   // Use the map (and not 'this') to compute the size tag, since
   // AsciiStringSize is called during GC when maps are encoded.
-  length >>= shape.size_tag() + kLongLengthShift;
+  length >>= StringShape(instance_type).size_tag() + kLongLengthShift;
 
   return SizeFor(length);
 }

src/objects.h

@@ -3139,13 +3139,9 @@ class StringHasher {
 // to be passed by value and is immutable, but be aware that flattening a
 // string can potentially alter its shape.  Also be aware that a GC caused by
 // something else can alter the shape of a string due to ConsString
-// shortcutting.
-//
-// Most of the methods designed to interrogate a string as to its exact nature
-// have been made into methods on StringShape in order to encourage the use of
-// StringShape.  The String class has both a length() and a length(StringShape)
-// operation.  The former is simpler to type, but the latter is faster if you
-// need the StringShape for some other operation immediately before or after.
+// shortcutting.  Keeping these restrictions in mind has proven to be error-
+// prone and so we no longer put StringShapes in variables unless there is a
+// concrete performance benefit at that particular point in the code.
 class StringShape BASE_EMBEDDED {
  public:
   inline explicit StringShape(String* s);
@@ -3194,9 +3190,6 @@ class StringShape BASE_EMBEDDED {
 class String: public HeapObject {
  public:
   // Get and set the length of the string.
-  // Fast version.
-  inline int length(StringShape shape);
-  // Easy version.
   inline int length();
   inline void set_length(int value);
 
@@ -3208,22 +3201,22 @@ class String: public HeapObject {
   inline void set_length_field(uint32_t value);
 
   // Get and set individual two byte chars in the string.
-  inline void Set(StringShape shape, int index, uint16_t value);
+  inline void Set(int index, uint16_t value);
   // Get individual two byte char in the string.  Repeated calls
   // to this method are not efficient unless the string is flat.
-  inline uint16_t Get(StringShape shape, int index);
+  inline uint16_t Get(int index);
 
   // Try to flatten the top level ConsString that is hiding behind this
   // string.  This is a no-op unless the string is a ConsString or a
   // SlicedString.  Flatten mutates the ConsString and might return a
   // failure.
-  Object* TryFlatten(StringShape shape);
+  Object* TryFlatten();
 
   // Try to flatten the string.  Checks first inline to see if it is necessary.
   // Do not handle allocation failures.  After calling TryFlattenIfNotFlat, the
   // string could still be a ConsString, in which case a failure is returned.
   // Use FlattenString from Handles.cc to be sure to flatten.
-  inline Object* TryFlattenIfNotFlat(StringShape shape);
+  inline Object* TryFlattenIfNotFlat();
 
   Vector<const char> ToAsciiVector();
   Vector<const uc16> ToUC16Vector();
@@ -3302,7 +3295,7 @@ class String: public HeapObject {
   void StringPrint();
   void StringVerify();
 #endif
-  inline bool IsFlat(StringShape shape);
+  inline bool IsFlat();
 
   // Layout description.
   static const int kLengthOffset = HeapObject::kHeaderSize;
@@ -3364,7 +3357,6 @@ class String: public HeapObject {
   // Helper function for flattening strings.
   template <typename sinkchar>
   static void WriteToFlat(String* source,
-                          StringShape shape,
                           sinkchar* sink,
                           int from,
                           int to);
@@ -3405,9 +3397,7 @@ class String: public HeapObject {
  private:
   // Slow case of String::Equals.  This implementation works on any strings
   // but it is most efficient on strings that are almost flat.
-  bool SlowEquals(StringShape this_shape,
-                  String* other,
-                  StringShape other_shape);
+  bool SlowEquals(String* other);
 
   // Slow case of AsArrayIndex.
   bool SlowAsArrayIndex(uint32_t* index);
@@ -3454,7 +3444,7 @@ class SeqAsciiString: public SeqString {
   // Garbage collection support.  This method is called by the
   // garbage collector to compute the actual size of an AsciiString
   // instance.
-  inline int SeqAsciiStringSize(StringShape shape);
+  inline int SeqAsciiStringSize(InstanceType instance_type);
 
   // Computes the size for an AsciiString instance of a given length.
   static int SizeFor(int length) {
@@ -3499,7 +3489,7 @@ class SeqTwoByteString: public SeqString {
   // Garbage collection support.  This method is called by the
   // garbage collector to compute the actual size of a TwoByteString
   // instance.
-  inline int SeqTwoByteStringSize(StringShape shape);
+  inline int SeqTwoByteStringSize(InstanceType instance_type);
 
   // Computes the size for a TwoByteString instance of a given length.
   static int SizeFor(int length) {

src/parser.cc

@@ -1097,11 +1097,10 @@ FunctionLiteral* Parser::ParseProgram(Handle<String> source,
   ZoneScope zone_scope(DONT_DELETE_ON_EXIT);
 
   HistogramTimerScope timer(&Counters::parse);
-  StringShape shape(*source);
-  Counters::total_parse_size.Increment(source->length(shape));
+  Counters::total_parse_size.Increment(source->length());
 
   // Initialize parser state.
-  source->TryFlattenIfNotFlat(shape);
+  source->TryFlattenIfNotFlat();
   scanner_.Init(source, stream, 0);
   ASSERT(target_stack_ == NULL);
 
@@ -1150,9 +1149,8 @@ FunctionLiteral* Parser::ParseLazy(Handle<String> source,
                                    bool is_expression) {
   ZoneScope zone_scope(DONT_DELETE_ON_EXIT);
   HistogramTimerScope timer(&Counters::parse_lazy);
-  source->TryFlattenIfNotFlat(StringShape(*source));
-  StringShape shape(*source);
-  Counters::total_parse_size.Increment(source->length(shape));
+  source->TryFlattenIfNotFlat();
+  Counters::total_parse_size.Increment(source->length());
   SafeStringInputBuffer buffer(source.location());
 
   // Initialize parser state.

src/prettyprinter.cc

@@ -517,10 +517,9 @@ void PrettyPrinter::PrintLiteral(Handle<Object> value, bool quote) {
   Object* object = *value;
   if (object->IsString()) {
     String* string = String::cast(object);
-    StringShape shape(string);
     if (quote) Print("\"");
-    for (int i = 0; i < string->length(shape); i++) {
-      Print("%c", string->Get(shape, i));
+    for (int i = 0; i < string->length(); i++) {
+      Print("%c", string->Get(i));
     }
     if (quote) Print("\"");
   } else if (object == Heap::null_value()) {

src/regexp-macro-assembler-ia32.cc

@@ -985,16 +985,14 @@ RegExpMacroAssemblerIA32::Result RegExpMacroAssemblerIA32::Match(
     int* offsets_vector,
     int offsets_vector_length,
     int previous_index) {
-  StringShape shape(*subject);
-
   // Character offsets into string.
   int start_offset = previous_index;
-  int end_offset = subject->length(shape);
+  int end_offset = subject->length();
 
-  if (shape.IsCons()) {
+  if (StringShape(*subject).IsCons()) {
     subject =
       Handle<String>(String::cast(ConsString::cast(*subject)->first()));
-  } else if (shape.IsSliced()) {
+  } else if (StringShape(*subject).IsSliced()) {
     SlicedString* slice = SlicedString::cast(*subject);
     start_offset += slice->start();
     end_offset += slice->start();
@@ -1002,13 +1000,12 @@ RegExpMacroAssemblerIA32::Result RegExpMacroAssemblerIA32::Match(
   }
 
   // String is now either Sequential or External
-  StringShape flatshape(*subject);
-  bool is_ascii = flatshape.IsAsciiRepresentation();
+  bool is_ascii = StringShape(*subject).IsAsciiRepresentation();
   int char_size_shift = is_ascii ? 0 : 1;
 
   RegExpMacroAssemblerIA32::Result res;
 
-  if (flatshape.IsExternal()) {
+  if (StringShape(*subject).IsExternal()) {
     const byte* address;
     if (is_ascii) {
       ExternalAsciiString* ext = ExternalAsciiString::cast(*subject);

test/cctest/test-heap.cc

@@ -254,10 +254,9 @@ TEST(GarbageCollection) {
 
 static void VerifyStringAllocation(const char* string) {
   String* s = String::cast(Heap::AllocateStringFromUtf8(CStrVector(string)));
-  StringShape shape(s);
-  CHECK_EQ(static_cast<int>(strlen(string)), s->length(shape));
-  for (int index = 0; index < s->length(shape); index++) {
-    CHECK_EQ(static_cast<uint16_t>(string[index]), s->Get(shape, index));  }
+  CHECK_EQ(static_cast<int>(strlen(string)), s->length());
+  for (int index = 0; index < s->length(); index++) {
+    CHECK_EQ(static_cast<uint16_t>(string[index]), s->Get(index));  }
 }
 
 

test/cctest/test-regexp.cc

@@ -643,8 +643,7 @@ TEST(MacroAssembler) {
 
   Handle<String> f1 =
       Factory::NewStringFromAscii(CStrVector("foobar"));
-  Handle<String> f1_16 = RegExpImpl::StringToTwoByte(f1);
-  CHECK(IrregexpInterpreter::Match(array, f1_16, captures, 0));
+  CHECK(IrregexpInterpreter::Match(array, f1, captures, 0));
   CHECK_EQ(0, captures[0]);
   CHECK_EQ(3, captures[1]);
   CHECK_EQ(1, captures[2]);
@@ -653,8 +652,7 @@ TEST(MacroAssembler) {
 
   Handle<String> f2 =
       Factory::NewStringFromAscii(CStrVector("barfoo"));
-  Handle<String> f2_16 = RegExpImpl::StringToTwoByte(f2);
-  CHECK(!IrregexpInterpreter::Match(array, f2_16, captures, 0));
+  CHECK(!IrregexpInterpreter::Match(array, f2, captures, 0));
   CHECK_EQ(42, captures[0]);
 }
 

test/cctest/test-strings.cc

@@ -62,8 +62,7 @@ static void InitializeBuildingBlocks(
         building_blocks[i] =
             Factory::NewStringFromTwoByte(Vector<const uc16>(buf, len));
         for (int j = 0; j < len; j++) {
-          StringShape shape(*building_blocks[i]);
-          CHECK_EQ(buf[j], building_blocks[i]->Get(shape, j));
+          CHECK_EQ(buf[j], building_blocks[i]->Get(j));
         }
         break;
       }
@@ -75,8 +74,7 @@ static void InitializeBuildingBlocks(
         building_blocks[i] =
             Factory::NewStringFromAscii(Vector<const char>(buf, len));
         for (int j = 0; j < len; j++) {
-          StringShape shape(*building_blocks[i]);
-          CHECK_EQ(buf[j], building_blocks[i]->Get(shape, j));
+          CHECK_EQ(buf[j], building_blocks[i]->Get(j));
         }
         break;
       }
@@ -101,8 +99,7 @@ static void InitializeBuildingBlocks(
         Resource* resource = new Resource(Vector<const uc16>(buf, len));
         building_blocks[i] = Factory::NewExternalStringFromTwoByte(resource);
         for (int j = 0; j < len; j++) {
-          StringShape shape(*building_blocks[i]);
-          CHECK_EQ(buf[j], building_blocks[i]->Get(shape, j));
+          CHECK_EQ(buf[j], building_blocks[i]->Get(j));
         }
         break;
       }
@@ -114,8 +111,7 @@ static void InitializeBuildingBlocks(
         building_blocks[i] =
             Factory::NewStringFromAscii(Vector<const char>(buf, len));
         for (int j = 0; j < len; j++) {
-          StringShape shape(*building_blocks[i]);
-          CHECK_EQ(buf[j], building_blocks[i]->Get(shape, j));
+          CHECK_EQ(buf[j], building_blocks[i]->Get(j));
         }
         DeleteArray<char>(buf);
         break;
@@ -132,9 +128,7 @@ static Handle<String> ConstructLeft(
   for (int i = 0; i < depth; i++) {
     answer = Factory::NewConsString(
         answer,
-        StringShape(*answer),
-        building_blocks[i % NUMBER_OF_BUILDING_BLOCKS],
-        StringShape(*building_blocks[i % NUMBER_OF_BUILDING_BLOCKS]));
+        building_blocks[i % NUMBER_OF_BUILDING_BLOCKS]);
   }
   return answer;
 }
@@ -147,9 +141,7 @@ static Handle<String> ConstructRight(
   for (int i = depth - 1; i >= 0; i--) {
     answer = Factory::NewConsString(
         building_blocks[i % NUMBER_OF_BUILDING_BLOCKS],
-        StringShape(*building_blocks[i % NUMBER_OF_BUILDING_BLOCKS]),
-        answer,
-        StringShape(*answer));
+        answer);
   }
   return answer;
 }
@@ -166,19 +158,13 @@ static Handle<String> ConstructBalancedHelper(
   if (to - from == 2) {
     return Factory::NewConsString(
         building_blocks[from % NUMBER_OF_BUILDING_BLOCKS],
-        StringShape(*building_blocks[from % NUMBER_OF_BUILDING_BLOCKS]),
-        building_blocks[(from+1) % NUMBER_OF_BUILDING_BLOCKS],
-        StringShape(*building_blocks[(from+1) % NUMBER_OF_BUILDING_BLOCKS]));
+        building_blocks[(from+1) % NUMBER_OF_BUILDING_BLOCKS]);
   }
   Handle<String> part1 =
     ConstructBalancedHelper(building_blocks, from, from + ((to - from) / 2));
   Handle<String> part2 =
     ConstructBalancedHelper(building_blocks, from + ((to - from) / 2), to);
-  return Factory::NewConsString(
-      part1,
-      StringShape(*part1),
-      part2,
-      StringShape(*part2));
+  return Factory::NewConsString(part1, part2);
 }
 
 
@@ -216,8 +202,8 @@ static void TraverseFirst(Handle<String> s1, Handle<String> s2, int chars) {
     CHECK_EQ(c, buffer2.GetNext());
     i++;
   }
-  s1->Get(StringShape(*s1), s1->length() - 1);
-  s2->Get(StringShape(*s2), s2->length() - 1);
+  s1->Get(s1->length() - 1);
+  s2->Get(s2->length() - 1);
 }
 
 
@@ -299,19 +285,13 @@ static Handle<String> ConstructSliceTree(
     Handle<String> rhs = building_blocks[(from+1) % NUMBER_OF_BUILDING_BLOCKS];
     if (gen() % 2 == 0)
       rhs = SliceOf(rhs);
-    return Factory::NewConsString(lhs,
-                                  StringShape(*lhs),
-                                  rhs,
-                                  StringShape(*rhs));
+    return Factory::NewConsString(lhs, rhs);
   }
   Handle<String> part1 =
     ConstructBalancedHelper(building_blocks, from, from + ((to - from) / 2));
   Handle<String> part2 =
     ConstructBalancedHelper(building_blocks, from + ((to - from) / 2), to);
-  Handle<String> branch = Factory::NewConsString(part1,
-                                                 StringShape(*part1),
-                                                 part2,
-                                                 StringShape(*part2));
+  Handle<String> branch = Factory::NewConsString(part1, part2);
   if (gen() % 2 == 0)
     return branch;
   return(SliceOf(branch));
@@ -351,15 +331,9 @@ TEST(DeepAscii) {
       Factory::NewStringFromAscii(Vector<const char>(foo, DEEP_ASCII_DEPTH));
   Handle<String> foo_string = Factory::NewStringFromAscii(CStrVector("foo"));
   for (int i = 0; i < DEEP_ASCII_DEPTH; i += 10) {
-    string = Factory::NewConsString(string,
-                                    StringShape(*string),
-                                    foo_string,
-                                    StringShape(*foo_string));
+    string = Factory::NewConsString(string, foo_string);
   }
-  Handle<String> flat_string = Factory::NewConsString(string,
-                                                      StringShape(*string),
-                                                      foo_string,
-                                                      StringShape(*foo_string));
+  Handle<String> flat_string = Factory::NewConsString(string, foo_string);
   FlattenString(flat_string);
 
   for (int i = 0; i < 500; i++) {