From 164e5b580c3e353eb00c0ae59879d819492c6d7e Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Thu, 3 Apr 2014 12:30:37 +0000 Subject: [PATCH] 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 --- src/api.cc | 15 +++---- src/factory.cc | 8 ++-- src/factory.h | 9 +++- src/func-name-inferrer.cc | 7 ++- src/handles.h | 1 + src/isolate.h | 2 +- src/json-stringifier.h | 10 +++-- src/parser.cc | 8 ++-- src/runtime.cc | 93 +++++++++++++++++++++------------------ src/uri.h | 9 ++-- test/cctest/test-heap-profiler.cc | 3 +- test/cctest/test-strings.cc | 40 ++++++++++------- 12 files changed, 117 insertions(+), 88 deletions(-) diff --git a/src/api.cc b/src/api.cc index 1386a3a..1e9e862 100644 --- a/src/api.cc +++ b/src/api.cc @@ -5459,10 +5459,9 @@ Local v8::String::Concat(Handle left, Handle right) { LOG_API(isolate, "String::New(char)"); ENTER_V8(isolate); i::Handle right_string = Utils::OpenHandle(*right); - i::Handle 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 result = isolate->factory()->NewConsString( + left_string, right_string).ToHandleChecked(); return Utils::ToLocal(result); } @@ -7033,15 +7032,15 @@ Handle CpuProfileNode::GetFunctionName() const { i::Isolate* isolate = i::Isolate::Current(); const i::ProfileNode* node = reinterpret_cast(this); const i::CodeEntry* entry = node->entry(); + i::Handle name = + isolate->factory()->InternalizeUtf8String(entry->name()); if (!entry->has_name_prefix()) { - return ToApiHandle( - isolate->factory()->InternalizeUtf8String(entry->name())); + return ToApiHandle(name); } else { + // We do not expect this to fail. Change this if it does. i::Handle 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(cons); } } diff --git a/src/factory.cc b/src/factory.cc index 6218c64..0638271 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -336,8 +336,8 @@ Handle Factory::NewRawConsString(String::Encoding encoding) { } -Handle Factory::NewConsString(Handle left, - Handle right) { +MaybeHandle Factory::NewConsString(Handle left, + Handle right) { int left_length = left->length(); if (left_length == 0) return right; int right_length = right->length(); @@ -354,8 +354,8 @@ Handle Factory::NewConsString(Handle 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::null(); + return isolate()->Throw( + isolate()->factory()->NewInvalidStringLengthError()); } bool left_is_one_byte = left->IsOneByteRepresentation(); diff --git a/src/factory.h b/src/factory.h index 61105a7..df83d43 100644 --- 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 NewConsString(Handle left, - Handle right); + MUST_USE_RESULT MaybeHandle NewConsString(Handle left, + Handle right); Handle NewRawConsString(String::Encoding encoding); @@ -436,6 +436,11 @@ class Factory V8_FINAL { Vector< Handle > args); Handle NewRangeError(Handle message); + Handle NewInvalidStringLengthError() { + return NewRangeError("invalid_string_length", + HandleVector(NULL, 0)); + } + Handle NewSyntaxError(const char* message, Handle args); Handle NewSyntaxError(Handle message); diff --git a/src/func-name-inferrer.cc b/src/func-name-inferrer.cc index 441113b..e85e895 100644 --- a/src/func-name-inferrer.cc +++ b/src/func-name-inferrer.cc @@ -86,10 +86,9 @@ Handle FuncNameInferrer::MakeNameFromStackHelper(int pos, Handle name = names_stack_.at(pos).name; if (prev->length() + name->length() + 1 > String::kMaxLength) return prev; Factory* factory = isolate()->factory(); - Handle 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 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 --- 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 class MaybeHandle { diff --git a/src/isolate.h b/src/isolate.h index 2d7995c..eabe58b 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -179,7 +179,7 @@ typedef ZoneList > 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 --- 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 cons = factory_->NewConsString(accumulator(), result_string); - RETURN_IF_EMPTY_HANDLE_VALUE(isolate_, cons, EXCEPTION); + Handle 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 --- 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 tempstr = - heap_factory->NewConsString(heap_factory->dot_for_string(), name); - RETURN_IF_EMPTY_HANDLE_VALUE(isolate(), tempstr, 0); + Handle tempstr; + ASSIGN_RETURN_ON_EXCEPTION_VALUE( + isolate(), tempstr, + heap_factory->NewConsString(heap_factory->dot_for_string(), name), + 0); Handle 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 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -4235,35 +4235,34 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_StringReplaceGlobalRegExpWithString) { } -Handle StringReplaceOneCharWithString(Isolate* isolate, - Handle subject, - Handle search, - Handle replace, - bool* found, - int recursion_limit) { - if (recursion_limit == 0) return Handle::null(); +// This may return an empty MaybeHandle if an exception is thrown or +// we abort due to reaching the recursion limit. +MaybeHandle StringReplaceOneCharWithString(Isolate* isolate, + Handle subject, + Handle search, + Handle replace, + bool* found, + int recursion_limit) { + if (recursion_limit == 0) return MaybeHandle(); + recursion_limit--; if (subject->IsConsString()) { ConsString* cons = ConsString::cast(*subject); Handle first = Handle(cons->first()); Handle second = Handle(cons->second()); - Handle new_first = - StringReplaceOneCharWithString(isolate, - first, - search, - replace, - found, - recursion_limit - 1); - if (new_first.is_null()) return new_first; + Handle new_first; + if (!StringReplaceOneCharWithString( + isolate, first, search, replace, found, recursion_limit) + .ToHandle(&new_first)) { + return MaybeHandle(); + } if (*found) return isolate->factory()->NewConsString(new_first, second); - Handle new_second = - StringReplaceOneCharWithString(isolate, - second, - search, - replace, - found, - recursion_limit - 1); - if (new_second.is_null()) return new_second; + Handle new_second; + if (!StringReplaceOneCharWithString( + isolate, second, search, replace, found, recursion_limit) + .ToHandle(&new_second)) { + return MaybeHandle(); + } if (*found) return isolate->factory()->NewConsString(first, new_second); return subject; @@ -4272,8 +4271,11 @@ Handle StringReplaceOneCharWithString(Isolate* isolate, if (index == -1) return subject; *found = true; Handle first = isolate->factory()->NewSubString(subject, 0, index); - Handle cons1 = isolate->factory()->NewConsString(first, replace); - RETURN_IF_EMPTY_HANDLE_VALUE(isolate, cons1, Handle()); + Handle cons1; + ASSIGN_RETURN_ON_EXCEPTION( + isolate, cons1, + isolate->factory()->NewConsString(first, replace), + String); Handle 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 result = StringReplaceOneCharWithString(isolate, - subject, - search, - replace, - &found, - kRecursionLimit); - if (!result.is_null()) return *result; + Handle 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 = FlattenGetString(source); ASSERT(string->IsFlat()); - return string->IsOneByteRepresentationUnderneath() - ? *URIUnescape::Unescape(isolate, source) - : *URIUnescape::Unescape(isolate, source); + Handle result; + ASSIGN_RETURN_FAILURE_ON_EXCEPTION( + isolate, result, + string->IsOneByteRepresentationUnderneath() + ? URIUnescape::Unescape(isolate, source) + : URIUnescape::Unescape(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 result = isolate->factory()->NewConsString(str1, str2); - RETURN_IF_EMPTY_HANDLE(isolate, result); + Handle 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 --- a/src/uri.h +++ b/src/uri.h @@ -61,13 +61,13 @@ Vector GetCharVector(Handle string) { class URIUnescape : public AllStatic { public: template - static Handle Unescape(Isolate* isolate, Handle source); + static MaybeHandle Unescape(Isolate* isolate, Handle source); private: static const signed char kHexValue['g']; template - static Handle UnescapeSlow( + static MaybeHandle UnescapeSlow( Isolate* isolate, Handle string, int start_index); static INLINE(int TwoDigitHex(uint16_t character1, uint16_t character2)); @@ -91,7 +91,8 @@ const signed char URIUnescape::kHexValue[] = { template -Handle URIUnescape::Unescape(Isolate* isolate, Handle source) { +MaybeHandle URIUnescape::Unescape(Isolate* isolate, + Handle source) { int index; { DisallowHeapAllocation no_allocation; StringSearch search(isolate, STATIC_ASCII_VECTOR("%")); @@ -103,7 +104,7 @@ Handle URIUnescape::Unescape(Isolate* isolate, Handle source) { template -Handle URIUnescape::UnescapeSlow( +MaybeHandle URIUnescape::UnescapeSlow( Isolate* isolate, Handle 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 --- 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 second = factory->NewStringFromAscii(i::CStrVector("0123456789")); - i::Handle cons_string = factory->NewConsString(first, second); + i::Handle cons_string = + factory->NewConsString(first, second).ToHandleChecked(); global->SetInternalField(0, v8::ToApiHandle(cons_string)); diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index 6ff5200..f4b3719 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -447,7 +447,7 @@ static Handle ConstructRandomString(ConsStringGenerationData* data, left = ConstructRandomString(data, max_recursion - 1); } // Build the cons string. - Handle root = factory->NewConsString(left, right); + Handle 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 ConstructLeft( data->stats_.leaves_++; for (int i = 0; i < depth; i++) { Handle block = data->block(i); - Handle next = factory->NewConsString(answer, block); + Handle 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 ConstructRight( data->stats_.leaves_++; for (int i = depth - 1; i >= 0; i--) { Handle block = data->block(i); - Handle next = factory->NewConsString(block, answer); + Handle 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 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 part1 = ConstructBalancedHelper(data, from, from + ((to - from) / 2)); @@ -516,7 +519,7 @@ static Handle 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 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 BuildEdgeCaseConsString( data->stats_.empty_leaves_ += 1; { Handle 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 BuildEdgeCaseConsString( data->stats_.left_traversals_ += 1; { Handle 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 BuildEdgeCaseConsString( data->stats_.right_traversals_ += 1; { Handle left = - factory->NewConsString(data->block(0), data->block(1)); + factory->NewConsString(data->block(0), data->block(1)) + .ToHandleChecked(); FlattenString(left); Handle 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(foo, DEEP_ASCII_DEPTH)); Handle 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 flat_string = factory->NewConsString(string, foo_string); + Handle 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 = factory->NewStringFromAscii(CStrVector("parentparentparent")); - Handle parent = factory->NewConsString(string, string); + Handle parent = + factory->NewConsString(string, string).ToHandleChecked(); CHECK(parent->IsConsString()); CHECK(!parent->IsFlat()); Handle slice = factory->NewSubString(parent, 1, 25); -- 2.7.4