Removed some unsafe uses of StringShape.
authorerik.corry@gmail.com <erik.corry@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 5 Nov 2008 10:26:08 +0000 (10:26 +0000)
committererik.corry@gmail.com <erik.corry@gmail.com@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 5 Nov 2008 10:26:08 +0000 (10:26 +0000)
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

14 files changed:
src/compiler.cc
src/conversions.cc
src/factory.cc
src/factory.h
src/handles.cc
src/heap.cc
src/heap.h
src/jsregexp.cc
src/log.cc
src/objects.cc
src/objects.h
src/parser.cc
src/runtime.cc
test/cctest/test-strings.cc

index 53c1a662a2eb4055c2bc8b0ae728899c82e9093d..2a2bb87269b75e29a152eb410c42fb0ecffc166d 100644 (file)
@@ -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);
 
index 20a394a5894bbab05028c3d5d15e914229d3fa42..4b4faa551ffc87a70e49414db043f1e7c7974226 100644 (file)
@@ -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));
 }
 
index 3b3c2c3e57bddb01628d9ea34a1533ee285d7374..209cd4d3370b4bd922404abd0fc196fef8cbd1d1 100644 (file)
@@ -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);
 }
 
 
index 67fed5746803b20b29d2e399aef167a2e11ac76e..89e2d698fcd61e82f7edcaa4c9e99d21b3e99b45 100644 (file)
@@ -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);
 
index 70433192ce627b56eb7fee29b06c52f8dd73f5dd..ada10f1700fc0e64309978c1c26e6e0e450975f3 100644 (file)
@@ -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);
 }
 
 
index 805ca0948ce90778f7515f5eb305bb7a9cf62e29..e62234f08d7478c5fc8d23b80712008b4003d5c6 100644 (file)
@@ -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.
index dc50c73c91c3a62d9291789dd52bcdc869d4c9af..d89caf93d1eee701cf1d2f780af24dad69808cfe 100644 (file)
@@ -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);
 
index 1ad33b262bb23a1c348d32f01dcb718312353c75..2dd21bd71e25758158154b211a295c1373474e81 100644 (file)
@@ -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()) :
index c7b1539fe7dc9ba192ed948222f062d37d859b83..bd85fea9fc487be38ffde6431886f0e650af59b8 100644 (file)
@@ -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
 }
 
index 8421856753431677a9a377024ea3709d2e8c1aa7..00d4406d7cad88abca9e2468c7f73cc9e9fd8bd9 100644 (file)
@@ -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;
   }
index a29182264e6a70564b92adf09ce783e80dca2714..82edc53d4b03b20d1709573b375b12c8055f1d0b 100644 (file)
@@ -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);
 
index ef92e3c2886ac4f76ab60c5cc8a8ecc976f9b03c..bcc439f4ceb16dacec51ed247b093028f0c1557d 100644 (file)
@@ -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());
 
index cbf45f3c70ddd5a8a37ce9e0b1d1e2811bd29618..132ccb47b9f820e192ef0520850d6eedd0c776f2 100644 (file)
@@ -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);
 }
 
 
index ec080e245a19f456673f4d323bb94111a7b5d3c1..c5e73521b9ebf38f4e6335a81721396911e277df 100644 (file)
@@ -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);
 }