Removed some unsafe uses of StringShape.

Simplified some uses of StringShape.
Removed unused function SlicedStringFlatten.
Review URL: http://codereview.chromium.org/9408

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

src/compiler.cc

@@ -209,8 +209,7 @@ Handle<JSFunction> Compiler::Compile(Handle<String> source,
 Handle<JSFunction> Compiler::CompileEval(Handle<String> source,
                                          int line_offset,
                                          bool is_global) {
-  StringShape source_shape(*source);
-  int source_length = source->length(source_shape);
+  int source_length = source->length();
   Counters::total_eval_size.Increment(source_length);
   Counters::total_compile_size.Increment(source_length);
 

src/conversions.cc

@@ -121,15 +121,14 @@ static inline bool SubStringEquals(const char* str,
 
 
 static inline bool SubStringEquals(String* str, int index, const char* other) {
-  StringShape shape(str);
   HandleScope scope;
-  int str_length = str->length(shape);
+  int str_length = str->length();
   int other_length = strlen(other);
   int end = index + other_length < str_length ?
             index + other_length :
             str_length;
   Handle<String> slice =
-      Factory::NewStringSlice(Handle<String>(str), shape, index, end);
+      Factory::NewStringSlice(Handle<String>(str), index, end);
   return slice->IsEqualTo(Vector<const char>(other, other_length));
 }
 

src/factory.cc

@@ -95,19 +95,14 @@ Handle<String> Factory::NewConsString(Handle<String> first,
                                       StringShape second_shape) {
   if (first->length(first_shape) == 0) return second;
   if (second->length(second_shape) == 0) return first;
-  CALL_HEAP_FUNCTION(Heap::AllocateConsString(*first,
-                                              first_shape,
-                                              *second,
-                                              second_shape),
-                     String);
+  CALL_HEAP_FUNCTION(Heap::AllocateConsString(*first, *second), String);
 }
 
 
 Handle<String> Factory::NewStringSlice(Handle<String> str,
-                                       StringShape shape,
                                        int begin,
                                        int end) {
-  CALL_HEAP_FUNCTION(str->Slice(shape, begin, end), String);
+  CALL_HEAP_FUNCTION(str->Slice(begin, end), String);
 }
 
 

src/factory.h

@@ -105,7 +105,6 @@ class Factory : public AllStatic {
   // Create a new sliced string object which represents a substring of a
   // backing string.
   static Handle<String> NewStringSlice(Handle<String> str,
-                                       StringShape shape,
                                        int begin,
                                        int end);
 

src/handles.cc

@@ -217,7 +217,7 @@ Handle<Object> LookupSingleCharacterStringFromCode(uint32_t index) {
 
 
 Handle<String> SubString(Handle<String> str, int start, int end) {
-  CALL_HEAP_FUNCTION(str->Slice(StringShape(*str), start, end), String);
+  CALL_HEAP_FUNCTION(str->Slice(start, end), String);
 }
 
 

src/heap.cc

@@ -783,8 +783,7 @@ static inline bool IsShortcutCandidate(HeapObject* object, Map* map) {
   // is a candidate for being shortcut by the scavenger.
   ASSERT(object->map() == map);
   if (map->instance_type() >= FIRST_NONSTRING_TYPE) return false;
-  StringShape shape(map);
-  return (shape.representation_tag() == kConsStringTag) &&
+  return (StringShape(map).representation_tag() == kConsStringTag) &&
          (ConsString::cast(object)->unchecked_second() == Heap::empty_string());
 }
 
@@ -1347,9 +1346,9 @@ Object* Heap::AllocateSharedFunctionInfo(Object* name) {
 
 
 Object* Heap::AllocateConsString(String* first,
-                                 StringShape first_shape,
-                                 String* second,
-                                 StringShape second_shape) {
+                                 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 length = first_length + second_length;
@@ -1411,9 +1410,9 @@ Object* Heap::AllocateConsString(String* first,
 
 
 Object* Heap::AllocateSlicedString(String* buffer,
-                                   StringShape buffer_shape,
                                    int start,
                                    int end) {
+  StringShape buffer_shape(buffer);
   int length = end - start;
 
   // If the resulting string is small make a sub string.

src/heap.h

@@ -495,9 +495,7 @@ class Heap : public AllStatic {
   // failed.
   // Please note this does not perform a garbage collection.
   static Object* AllocateConsString(String* first,
-                                    StringShape first_shape,
-                                    String* second,
-                                    StringShape second_shape);
+                                    String* second);
 
   // Allocates a new sliced string object which is a slice of an underlying
   // string buffer stretching from the index start (inclusive) to the index
@@ -506,7 +504,6 @@ class Heap : public AllStatic {
   // failed.
   // Please note this does not perform a garbage collection.
   static Object* AllocateSlicedString(String* buffer,
-                                      StringShape buffer_shape,
                                       int start,
                                       int end);
 

src/jsregexp.cc

@@ -132,6 +132,7 @@ 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()) :

src/log.cc

@@ -431,8 +431,7 @@ void Logger::RegExpExecEvent(Handle<JSRegExp> regexp,
   LogRegExpSource(regexp);
   fprintf(logfile_, ",");
   LogString(input_string);
-  StringShape shape(*input_string);
-  fprintf(logfile_, ",%d..%d\n", start_index, input_string->length(shape));
+  fprintf(logfile_, ",%d..%d\n", start_index, input_string->length());
 #endif
 }
 

src/objects.cc

@@ -3717,21 +3717,6 @@ uint16_t ConsString::ConsStringGet(int index) {
 }
 
 
-Object* SlicedString::SlicedStringFlatten() {
-  // The SlicedString constructor should ensure that there are no
-  // SlicedStrings that are constructed directly on top of other
-  // SlicedStrings.
-  String* buf = String::cast(buffer());
-  StringShape buf_shape(buf);
-  ASSERT(!buf_shape.IsSliced());
-  if (buf_shape.IsCons()) {
-    Object* ok = buf->Flatten(buf_shape);
-    if (ok->IsFailure()) return ok;
-  }
-  return this;
-}
-
-
 template <typename sinkchar>
 void String::WriteToFlat(String* src,
                          StringShape src_shape,
@@ -3975,8 +3960,7 @@ bool String::SlowEquals(StringShape this_shape,
 
 
 bool String::MarkAsUndetectable() {
-  StringShape shape(this);
-  if (shape.IsSymbol()) return false;
+  if (StringShape(this).IsSymbol()) return false;
 
   Map* map = this->map();
   if (map == Heap::short_string_map()) {
@@ -4134,7 +4118,8 @@ uint32_t String::ComputeLengthAndHashField(unibrow::CharacterStream* buffer,
 }
 
 
-Object* String::Slice(StringShape shape, int start, int end) {
+Object* String::Slice(int start, int end) {
+  StringShape shape(this);
   if (start == 0 && end == length(shape)) return this;
   if (shape.representation_tag() == kSlicedStringTag) {
     // Translate slices of a SlicedString into slices of the
@@ -4142,11 +4127,10 @@ Object* String::Slice(StringShape shape, int start, int end) {
     SlicedString* str = SlicedString::cast(this);
     String* buf = str->buffer();
     return Heap::AllocateSlicedString(buf,
-                                      StringShape(buf),
                                       str->start() + start,
                                       str->start() + end);
   }
-  Object* result = Heap::AllocateSlicedString(this, shape, start, end);
+  Object* result = Heap::AllocateSlicedString(this, start, end);
   if (result->IsFailure()) {
     return result;
   }

src/objects.h

@@ -3024,7 +3024,9 @@ class StringHasher {
 // the shape of the string is given its own class so that it can be retrieved
 // once and used for several string operations.  A StringShape is small enough
 // to be passed by value and is immutable, but be aware that flattening a
-// string can potentially alter its shape.
+// 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
@@ -3116,7 +3118,7 @@ class String: public HeapObject {
   bool MarkAsUndetectable();
 
   // Slice the string and return a substring.
-  Object* Slice(StringShape shape, int from, int to);
+  Object* Slice(int from, int to);
 
   // String equality operations.
   inline bool Equals(String* other);
@@ -3471,9 +3473,6 @@ class SlicedString: public String {
   // Dispatched behavior.
   uint16_t SlicedStringGet(int index);
 
-  // Flatten any ConsString hiding behind this SlicedString.
-  Object* SlicedStringFlatten();
-
   // Casting.
   static inline SlicedString* cast(Object* obj);
 

src/parser.cc

@@ -790,8 +790,8 @@ FunctionLiteral* Parser::ParseLazy(Handle<String> source,
                                    bool is_expression) {
   ZoneScope zone_scope(DONT_DELETE_ON_EXIT);
   StatsRateScope timer(&Counters::parse_lazy);
+  source->TryFlatten(StringShape(*source));
   StringShape shape(*source);
-  source->TryFlatten(shape);
   Counters::total_parse_size.Increment(source->length(shape));
   SafeStringInputBuffer buffer(source.location());
 

src/runtime.cc

@@ -971,12 +971,12 @@ static Object* CharCodeAt(String* subject, Object* index) {
   // Flatten the string.  If someone wants to get a char at an index
   // in a cons string, it is likely that more indices will be
   // accessed.
+  subject->TryFlatten(StringShape(subject));
   StringShape shape(subject);
-  subject->TryFlatten(shape);  // shape no longer valid!
-  if (i >= static_cast<uint32_t>(subject->length(StringShape(subject)))) {
+  if (i >= static_cast<uint32_t>(subject->length(shape))) {
     return Heap::nan_value();
   }
-  return Smi::FromInt(subject->Get(StringShape(subject), i));
+  return Smi::FromInt(subject->Get(shape, i));
 }
 
 
@@ -1357,10 +1357,9 @@ int Runtime::StringMatch(Handle<String> sub,
                          int start_index) {
   ASSERT(0 <= start_index);
   StringShape sub_shape(*sub);
-  StringShape pat_shape(*pat);
   ASSERT(start_index <= sub->length(sub_shape));
 
-  int pattern_length = pat->length(pat_shape);
+  int pattern_length = pat->length();
   if (pattern_length == 0) return start_index;
 
   int subject_length = sub->length(sub_shape);
@@ -1370,6 +1369,7 @@ int Runtime::StringMatch(Handle<String> sub,
     FlattenString(sub);
     sub_shape = StringShape(*sub);
   }
+  StringShape pat_shape(*pat);
   // Searching for one specific character is common.  For one
   // character patterns linear search is necessary, so any smart
   // algorithm is unnecessary overhead.
@@ -1388,6 +1388,7 @@ int Runtime::StringMatch(Handle<String> sub,
   if (!pat->IsFlat(pat_shape)) {
     FlattenString(pat);
     pat_shape = StringShape(*pat);
+    sub_shape = StringShape(*sub);
   }
 
   AssertNoAllocation no_heap_allocation;  // ensure vectors stay valid
@@ -1522,12 +1523,10 @@ static Object* Runtime_StringSlice(Arguments args) {
   int start = FastD2I(from_number);
   int end = FastD2I(to_number);
 
-  StringShape shape(value);
-
   RUNTIME_ASSERT(end >= start);
   RUNTIME_ASSERT(start >= 0);
-  RUNTIME_ASSERT(end <= value->length(shape));
-  return value->Slice(shape, start, end);
+  RUNTIME_ASSERT(end <= value->length());
+  return value->Slice(start, end);
 }
 
 
@@ -1910,8 +1909,7 @@ static Object* Runtime_HasLocalProperty(Arguments args) {
     uint32_t index;
     if (key->AsArrayIndex(&index)) {
       String* string = String::cast(args[0]);
-      StringShape shape(string);
-      if (index < static_cast<uint32_t>(string->length(shape)))
+      if (index < static_cast<uint32_t>(string->length()))
         return Heap::true_value();
     }
   }
@@ -2684,10 +2682,8 @@ static Object* Runtime_StringAdd(Arguments args) {
 
   CONVERT_CHECKED(String, str1, args[0]);
   CONVERT_CHECKED(String, str2, args[1]);
-  StringShape shape1(str1);
-  StringShape shape2(str2);
-  int len1 = str1->length(shape1);
-  int len2 = str2->length(shape2);
+  int len1 = str1->length();
+  int len2 = str2->length();
   if (len1 == 0) return str2;
   if (len2 == 0) return str1;
   int length_sum = len1 + len2;
@@ -2697,7 +2693,7 @@ static Object* Runtime_StringAdd(Arguments args) {
     Top::context()->mark_out_of_memory();
     return Failure::OutOfMemoryException();
   }
-  return Heap::AllocateConsString(str1, shape1, str2, shape2);
+  return Heap::AllocateConsString(str1, str2);
 }
 
 

test/cctest/test-strings.cc

@@ -215,10 +215,8 @@ static void TraverseFirst(Handle<String> s1, Handle<String> s2, int chars) {
     CHECK_EQ(c, buffer2.GetNext());
     i++;
   }
-  StringShape shape1(*s1);
-  StringShape shape2(*s2);
-  s1->Get(shape1, s1->length(shape1) - 1);
-  s2->Get(shape2, s2->length(shape2) - 1);
+  s1->Get(StringShape(*s1), s1->length() - 1);
+  s2->Get(StringShape(*s2), s2->length() - 1);
 }
 
 
@@ -251,12 +249,10 @@ TEST(Traverse) {
   printf("7\n");
   Handle<String> right_deep_slice =
       Factory::NewStringSlice(left_deep_asymmetric,
-                              StringShape(*left_deep_asymmetric),
                               left_deep_asymmetric->length() - 1050,
                               left_deep_asymmetric->length() - 50);
   Handle<String> left_deep_slice =
       Factory::NewStringSlice(right_deep_asymmetric,
-                              StringShape(*right_deep_asymmetric),
                               right_deep_asymmetric->length() - 1050,
                               right_deep_asymmetric->length() - 50);
   printf("8\n");
@@ -283,7 +279,6 @@ static Handle<String> SliceOf(Handle<String> underlying) {
   int start = gen() % underlying->length();
   int end = start + gen() % (underlying->length() - start);
   return Factory::NewStringSlice(underlying,
-                                 StringShape(*underlying),
                                  start,
                                  end);
 }