Reland "Return MaybeHandle from NewConsString."

R=ishell@chromium.org

Review URL: https://codereview.chromium.org/223813002

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

diff --git a/src/api.cc b/src/api.cc
index 1386a3a..1e9e862 100644 (file)
--- a/src/api.cc
+++ b/src/api.cc
@@ -5459,10 +5459,9 @@ Local<String> v8::String::Concat(Handle<String> left, Handle<String> right) {
   LOG_API(isolate, "String::New(char)");
   ENTER_V8(isolate);
   i::Handle<i::String> right_string = Utils::OpenHandle(*right);
-  i::Handle<i::String> result = isolate->factory()->NewConsString(left_string,
-                                                                  right_string);
   // We do not expect this to fail. Change this if it does.
-  CHECK(!result.is_null());
+  i::Handle<i::String> result = isolate->factory()->NewConsString(
+      left_string, right_string).ToHandleChecked();
   return Utils::ToLocal(result);
 }
 
@@ -7033,15 +7032,15 @@ Handle<String> CpuProfileNode::GetFunctionName() const {
   i::Isolate* isolate = i::Isolate::Current();
   const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this);
   const i::CodeEntry* entry = node->entry();
+  i::Handle<i::String> name =
+      isolate->factory()->InternalizeUtf8String(entry->name());
   if (!entry->has_name_prefix()) {
-    return ToApiHandle<String>(
-        isolate->factory()->InternalizeUtf8String(entry->name()));
+    return ToApiHandle<String>(name);
   } else {
+    // We do not expect this to fail. Change this if it does.
     i::Handle<i::String> cons = isolate->factory()->NewConsString(
         isolate->factory()->InternalizeUtf8String(entry->name_prefix()),
-        isolate->factory()->InternalizeUtf8String(entry->name()));
-    // We do not expect this to fail. Change this if it does.
-    CHECK(!cons.is_null());
+        name).ToHandleChecked();
     return ToApiHandle<String>(cons);
   }
 }

diff --git a/src/factory.cc b/src/factory.cc
index 6218c64..0638271 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -336,8 +336,8 @@ Handle<ConsString> Factory::NewRawConsString(String::Encoding encoding) {
 }
 
 
-Handle<String> Factory::NewConsString(Handle<String> left,
-                                      Handle<String> right) {
+MaybeHandle<String> Factory::NewConsString(Handle<String> left,
+                                           Handle<String> right) {
   int left_length = left->length();
   if (left_length == 0) return right;
   int right_length = right->length();
@@ -354,8 +354,8 @@ Handle<String> Factory::NewConsString(Handle<String> left,
   // Make sure that an out of memory exception is thrown if the length
   // of the new cons string is too large.
   if (length > String::kMaxLength || length < 0) {
-    isolate()->ThrowInvalidStringLength();
-    return Handle<String>::null();
+    return isolate()->Throw<String>(
+        isolate()->factory()->NewInvalidStringLengthError());
   }
 
   bool left_is_one_byte = left->IsOneByteRepresentation();

diff --git a/src/factory.h b/src/factory.h
index 61105a7..df83d43 100644 (file)
--- a/src/factory.h
+++ b/src/factory.h
@@ -140,8 +140,8 @@ class Factory V8_FINAL {
       PretenureFlag pretenure = NOT_TENURED);
 
   // Create a new cons string object which consists of a pair of strings.
-  Handle<String> NewConsString(Handle<String> left,
-                               Handle<String> right);
+  MUST_USE_RESULT MaybeHandle<String> NewConsString(Handle<String> left,
+                                                    Handle<String> right);
 
   Handle<ConsString> NewRawConsString(String::Encoding encoding);
 
@@ -436,6 +436,11 @@ class Factory V8_FINAL {
                                Vector< Handle<Object> > args);
   Handle<Object> NewRangeError(Handle<String> message);
 
+  Handle<Object> NewInvalidStringLengthError() {
+    return NewRangeError("invalid_string_length",
+                         HandleVector<Object>(NULL, 0));
+  }
+
   Handle<Object> NewSyntaxError(const char* message, Handle<JSArray> args);
   Handle<Object> NewSyntaxError(Handle<String> message);
 

diff --git a/src/func-name-inferrer.cc b/src/func-name-inferrer.cc
index 441113b..e85e895 100644 (file)
--- a/src/func-name-inferrer.cc
+++ b/src/func-name-inferrer.cc
@@ -86,10 +86,9 @@ Handle<String> FuncNameInferrer::MakeNameFromStackHelper(int pos,
       Handle<String> name = names_stack_.at(pos).name;
       if (prev->length() + name->length() + 1 > String::kMaxLength) return prev;
       Factory* factory = isolate()->factory();
-      Handle<String> curr = factory->NewConsString(factory->dot_string(), name);
-      CHECK_NOT_EMPTY_HANDLE(isolate(), curr);
-      curr = factory->NewConsString(prev, curr);
-      CHECK_NOT_EMPTY_HANDLE(isolate(), curr);
+      Handle<String> curr =
+          factory->NewConsString(factory->dot_string(), name).ToHandleChecked();
+      curr = factory->NewConsString(prev, curr).ToHandleChecked();
       return MakeNameFromStackHelper(pos + 1, curr);
     } else {
       return MakeNameFromStackHelper(pos + 1, names_stack_.at(pos).name);

diff --git a/src/handles.h b/src/handles.h
index 5b5ed1b..7418468 100644 (file)
--- a/src/handles.h
+++ b/src/handles.h
@@ -37,6 +37,7 @@ namespace internal {
 // A Handle can be converted into a MaybeHandle. Converting a MaybeHandle
 // into a Handle requires checking that it does not point to NULL.  This
 // ensures NULL checks before use.
+// Do not use MaybeHandle as argument type.
 
 template<typename T>
 class MaybeHandle {

diff --git a/src/isolate.h b/src/isolate.h
index 2d7995c..eabe58b 100644 (file)
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -179,7 +179,7 @@ typedef ZoneList<Handle<Object> > ZoneObjectList;
 
 #define RETURN_ON_EXCEPTION_VALUE(isolate, dst, call, value)  \
   do {                                                             \
-    if (call.is_null()) {                                          \
+    if ((call).is_null()) {                                        \
       ASSERT((isolate)->has_pending_exception());                  \
       return value;                                                \
     }                                                              \

diff --git a/src/json-stringifier.h b/src/json-stringifier.h
index a083d48..f8bac94 100644 (file)
--- a/src/json-stringifier.h
+++ b/src/json-stringifier.h
@@ -498,8 +498,11 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeGeneric(
   part_length_ = kInitialPartLength;  // Allocate conservatively.
   Extend();             // Attach current part and allocate new part.
   // Attach result string to the accumulator.
-  Handle<String> cons = factory_->NewConsString(accumulator(), result_string);
-  RETURN_IF_EMPTY_HANDLE_VALUE(isolate_, cons, EXCEPTION);
+  Handle<String> cons;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate_, cons,
+      factory_->NewConsString(accumulator(), result_string),
+      EXCEPTION);
   set_accumulator(cons);
   return SUCCESS;
 }
@@ -728,7 +731,8 @@ void BasicJsonStringifier::Accumulate() {
     set_accumulator(factory_->empty_string());
     overflowed_ = true;
   } else {
-    set_accumulator(factory_->NewConsString(accumulator(), current_part_));
+    set_accumulator(factory_->NewConsString(accumulator(),
+                                            current_part_).ToHandleChecked());
   }
 }
 

diff --git a/src/parser.cc b/src/parser.cc
index 5fce1dd..d0e01fc 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2974,9 +2974,11 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) {
         // TODO(keuchel): Move the temporary variable to the block scope, after
         // implementing stack allocated block scoped variables.
         Factory* heap_factory = isolate()->factory();
-        Handle<String> tempstr =
-            heap_factory->NewConsString(heap_factory->dot_for_string(), name);
-        RETURN_IF_EMPTY_HANDLE_VALUE(isolate(), tempstr, 0);
+        Handle<String> tempstr;
+        ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+            isolate(), tempstr,
+            heap_factory->NewConsString(heap_factory->dot_for_string(), name),
+            0);
         Handle<String> tempname = heap_factory->InternalizeString(tempstr);
         Variable* temp = scope_->DeclarationScope()->NewTemporary(tempname);
         VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);

diff --git a/src/runtime.cc b/src/runtime.cc
index 4c0536e..12bce2c 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4235,35 +4235,34 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StringReplaceGlobalRegExpWithString) {
 }
 
 
-Handle<String> StringReplaceOneCharWithString(Isolate* isolate,
-                                              Handle<String> subject,
-                                              Handle<String> search,
-                                              Handle<String> replace,
-                                              bool* found,
-                                              int recursion_limit) {
-  if (recursion_limit == 0) return Handle<String>::null();
+// This may return an empty MaybeHandle if an exception is thrown or
+// we abort due to reaching the recursion limit.
+MaybeHandle<String> StringReplaceOneCharWithString(Isolate* isolate,
+                                                   Handle<String> subject,
+                                                   Handle<String> search,
+                                                   Handle<String> replace,
+                                                   bool* found,
+                                                   int recursion_limit) {
+  if (recursion_limit == 0) return MaybeHandle<String>();
+  recursion_limit--;
   if (subject->IsConsString()) {
     ConsString* cons = ConsString::cast(*subject);
     Handle<String> first = Handle<String>(cons->first());
     Handle<String> second = Handle<String>(cons->second());
-    Handle<String> new_first =
-        StringReplaceOneCharWithString(isolate,
-                                       first,
-                                       search,
-                                       replace,
-                                       found,
-                                       recursion_limit - 1);
-    if (new_first.is_null()) return new_first;
+    Handle<String> new_first;
+    if (!StringReplaceOneCharWithString(
+            isolate, first, search, replace, found, recursion_limit)
+            .ToHandle(&new_first)) {
+      return MaybeHandle<String>();
+    }
     if (*found) return isolate->factory()->NewConsString(new_first, second);
 
-    Handle<String> new_second =
-        StringReplaceOneCharWithString(isolate,
-                                       second,
-                                       search,
-                                       replace,
-                                       found,
-                                       recursion_limit - 1);
-    if (new_second.is_null()) return new_second;
+    Handle<String> new_second;
+    if (!StringReplaceOneCharWithString(
+            isolate, second, search, replace, found, recursion_limit)
+            .ToHandle(&new_second)) {
+      return MaybeHandle<String>();
+    }
     if (*found) return isolate->factory()->NewConsString(first, new_second);
 
     return subject;
@@ -4272,8 +4271,11 @@ Handle<String> StringReplaceOneCharWithString(Isolate* isolate,
     if (index == -1) return subject;
     *found = true;
     Handle<String> first = isolate->factory()->NewSubString(subject, 0, index);
-    Handle<String> cons1 = isolate->factory()->NewConsString(first, replace);
-    RETURN_IF_EMPTY_HANDLE_VALUE(isolate, cons1, Handle<String>());
+    Handle<String> cons1;
+    ASSIGN_RETURN_ON_EXCEPTION(
+        isolate, cons1,
+        isolate->factory()->NewConsString(first, replace),
+        String);
     Handle<String> second =
         isolate->factory()->NewSubString(subject, index + 1, subject->length());
     return isolate->factory()->NewConsString(cons1, second);
@@ -4292,20 +4294,20 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StringReplaceOneCharWithString) {
   // retry with a flattened subject string.
   const int kRecursionLimit = 0x1000;
   bool found = false;
-  Handle<String> result = StringReplaceOneCharWithString(isolate,
-                                                         subject,
-                                                         search,
-                                                         replace,
-                                                         &found,
-                                                         kRecursionLimit);
-  if (!result.is_null()) return *result;
+  Handle<String> result;
+  if (StringReplaceOneCharWithString(
+          isolate, subject, search, replace, &found, kRecursionLimit)
+          .ToHandle(&result)) {
+    return *result;
+  }
   if (isolate->has_pending_exception()) return Failure::Exception();
-  return *StringReplaceOneCharWithString(isolate,
-                                         FlattenGetString(subject),
-                                         search,
-                                         replace,
-                                         &found,
-                                         kRecursionLimit);
+
+  subject = FlattenGetString(subject);
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result,
+      StringReplaceOneCharWithString(
+          isolate, subject, search, replace, &found, kRecursionLimit));
+  return *result;
 }
 
 
@@ -6296,9 +6298,13 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_URIUnescape) {
   CONVERT_ARG_HANDLE_CHECKED(String, source, 0);
   Handle<String> string = FlattenGetString(source);
   ASSERT(string->IsFlat());
-  return string->IsOneByteRepresentationUnderneath()
-      ? *URIUnescape::Unescape<uint8_t>(isolate, source)
-      : *URIUnescape::Unescape<uc16>(isolate, source);
+  Handle<String> result;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result,
+      string->IsOneByteRepresentationUnderneath()
+            ? URIUnescape::Unescape<uint8_t>(isolate, source)
+            : URIUnescape::Unescape<uc16>(isolate, source));
+  return *result;
 }
 
 
@@ -7068,8 +7074,9 @@ RUNTIME_FUNCTION(MaybeObject*, RuntimeHidden_StringAdd) {
   CONVERT_ARG_HANDLE_CHECKED(String, str1, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, str2, 1);
   isolate->counters()->string_add_runtime()->Increment();
-  Handle<String> result = isolate->factory()->NewConsString(str1, str2);
-  RETURN_IF_EMPTY_HANDLE(isolate, result);
+  Handle<String> result;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result, isolate->factory()->NewConsString(str1, str2));
   return *result;
 }
 

diff --git a/src/uri.h b/src/uri.h
index 58509ce..70273fe 100644 (file)
--- a/src/uri.h
+++ b/src/uri.h
@@ -61,13 +61,13 @@ Vector<const uc16> GetCharVector(Handle<String> string) {
 class URIUnescape : public AllStatic {
  public:
   template<typename Char>
-  static Handle<String> Unescape(Isolate* isolate, Handle<String> source);
+  static MaybeHandle<String> Unescape(Isolate* isolate, Handle<String> source);
 
  private:
   static const signed char kHexValue['g'];
 
   template<typename Char>
-  static Handle<String> UnescapeSlow(
+  static MaybeHandle<String> UnescapeSlow(
       Isolate* isolate, Handle<String> string, int start_index);
 
   static INLINE(int TwoDigitHex(uint16_t character1, uint16_t character2));
@@ -91,7 +91,8 @@ const signed char URIUnescape::kHexValue[] = {
 
 
 template<typename Char>
-Handle<String> URIUnescape::Unescape(Isolate* isolate, Handle<String> source) {
+MaybeHandle<String> URIUnescape::Unescape(Isolate* isolate,
+                                          Handle<String> source) {
   int index;
   { DisallowHeapAllocation no_allocation;
     StringSearch<uint8_t, Char> search(isolate, STATIC_ASCII_VECTOR("%"));
@@ -103,7 +104,7 @@ Handle<String> URIUnescape::Unescape(Isolate* isolate, Handle<String> source) {
 
 
 template <typename Char>
-Handle<String> URIUnescape::UnescapeSlow(
+MaybeHandle<String> URIUnescape::UnescapeSlow(
     Isolate* isolate, Handle<String> string, int start_index) {
   bool one_byte = true;
   int length = string->length();

diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc
index 56c81de..c38af93 100644 (file)
--- a/test/cctest/test-heap-profiler.cc
+++ b/test/cctest/test-heap-profiler.cc
@@ -445,7 +445,8 @@ TEST(HeapSnapshotConsString) {
       factory->NewStringFromAscii(i::CStrVector("0123456789"));
   i::Handle<i::String> second =
       factory->NewStringFromAscii(i::CStrVector("0123456789"));
-  i::Handle<i::String> cons_string = factory->NewConsString(first, second);
+  i::Handle<i::String> cons_string =
+      factory->NewConsString(first, second).ToHandleChecked();
 
   global->SetInternalField(0, v8::ToApiHandle<v8::String>(cons_string));
 

diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc
index 6ff5200..f4b3719 100644 (file)
--- a/test/cctest/test-strings.cc
+++ b/test/cctest/test-strings.cc
@@ -447,7 +447,7 @@ static Handle<String> ConstructRandomString(ConsStringGenerationData* data,
     left = ConstructRandomString(data, max_recursion - 1);
   }
   // Build the cons string.
-  Handle<String> root = factory->NewConsString(left, right);
+  Handle<String> root = factory->NewConsString(left, right).ToHandleChecked();
   CHECK(root->IsConsString() && !root->IsFlat());
   // Special work needed for flat string.
   if (flat) {
@@ -467,7 +467,8 @@ static Handle<String> ConstructLeft(
   data->stats_.leaves_++;
   for (int i = 0; i < depth; i++) {
     Handle<String> block = data->block(i);
-    Handle<String> next = factory->NewConsString(answer, block);
+    Handle<String> next =
+        factory->NewConsString(answer, block).ToHandleChecked();
     if (next->IsConsString()) data->stats_.leaves_++;
     data->stats_.chars_ += block->length();
     answer = next;
@@ -485,7 +486,8 @@ static Handle<String> ConstructRight(
   data->stats_.leaves_++;
   for (int i = depth - 1; i >= 0; i--) {
     Handle<String> block = data->block(i);
-    Handle<String> next = factory->NewConsString(block, answer);
+    Handle<String> next =
+        factory->NewConsString(block, answer).ToHandleChecked();
     if (next->IsConsString()) data->stats_.leaves_++;
     data->stats_.chars_ += block->length();
     answer = next;
@@ -508,7 +510,8 @@ static Handle<String> ConstructBalancedHelper(
   if (to - from == 2) {
     data->stats_.chars_ += data->block(from)->length();
     data->stats_.chars_ += data->block(from+1)->length();
-    return factory->NewConsString(data->block(from), data->block(from+1));
+    return factory->NewConsString(data->block(from), data->block(from+1))
+        .ToHandleChecked();
   }
   Handle<String> part1 =
     ConstructBalancedHelper(data, from, from + ((to - from) / 2));
@@ -516,7 +519,7 @@ static Handle<String> ConstructBalancedHelper(
     ConstructBalancedHelper(data, from + ((to - from) / 2), to);
   if (part1->IsConsString()) data->stats_.left_traversals_++;
   if (part2->IsConsString()) data->stats_.right_traversals_++;
-  return factory->NewConsString(part1, part2);
+  return factory->NewConsString(part1, part2).ToHandleChecked();
 }
 
 
@@ -710,7 +713,8 @@ static Handle<String> BuildEdgeCaseConsString(
       data->stats_.chars_ += data->block(0)->length();
       data->stats_.chars_ += data->block(1)->length();
       data->stats_.leaves_ += 2;
-      return factory->NewConsString(data->block(0), data->block(1));
+      return factory->NewConsString(data->block(0), data->block(1))
+                 .ToHandleChecked();
     case 6:
       // Simple flattened tree.
       data->stats_.chars_ += data->block(0)->length();
@@ -719,7 +723,8 @@ static Handle<String> BuildEdgeCaseConsString(
       data->stats_.empty_leaves_ += 1;
       {
         Handle<String> string =
-            factory->NewConsString(data->block(0), data->block(1));
+            factory->NewConsString(data->block(0), data->block(1))
+                .ToHandleChecked();
         FlattenString(string);
         return string;
       }
@@ -733,9 +738,10 @@ static Handle<String> BuildEdgeCaseConsString(
       data->stats_.left_traversals_ += 1;
       {
         Handle<String> left =
-            factory->NewConsString(data->block(0), data->block(1));
+            factory->NewConsString(data->block(0), data->block(1))
+                .ToHandleChecked();
         FlattenString(left);
-        return factory->NewConsString(left, data->block(2));
+        return factory->NewConsString(left, data->block(2)).ToHandleChecked();
       }
     case 8:
       // Left node and right node flattened.
@@ -749,12 +755,14 @@ static Handle<String> BuildEdgeCaseConsString(
       data->stats_.right_traversals_ += 1;
       {
         Handle<String> left =
-            factory->NewConsString(data->block(0), data->block(1));
+            factory->NewConsString(data->block(0), data->block(1))
+                .ToHandleChecked();
         FlattenString(left);
         Handle<String> right =
-            factory->NewConsString(data->block(2), data->block(2));
+            factory->NewConsString(data->block(2), data->block(2))
+                .ToHandleChecked();
         FlattenString(right);
-        return factory->NewConsString(left, right);
+        return factory->NewConsString(left, right).ToHandleChecked();
       }
   }
   UNREACHABLE();
@@ -866,9 +874,10 @@ 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, foo_string);
+    string = factory->NewConsString(string, foo_string).ToHandleChecked();
   }
-  Handle<String> flat_string = factory->NewConsString(string, foo_string);
+  Handle<String> flat_string =
+      factory->NewConsString(string, foo_string).ToHandleChecked();
   FlattenString(flat_string);
 
   for (int i = 0; i < 500; i++) {
@@ -1092,7 +1101,8 @@ TEST(SliceFromCons) {
   v8::HandleScope scope(CcTest::isolate());
   Handle<String> string =
       factory->NewStringFromAscii(CStrVector("parentparentparent"));
-  Handle<String> parent = factory->NewConsString(string, string);
+  Handle<String> parent =
+      factory->NewConsString(string, string).ToHandleChecked();
   CHECK(parent->IsConsString());
   CHECK(!parent->IsFlat());
   Handle<String> slice = factory->NewSubString(parent, 1, 25);