[api] Remove unsafe ToLocal and ToMaybeLocal
Replace by `Utils::To(Maybe)Local` which does type inference, followed
by a cast, if needed.
R=leszeks@chromium.org
CC=jkummerow@chromium.org
Bug: 415850621
Change-Id: Ifffc2430b469fe32eaaf554d14529dd75d87cc7a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6531243
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100193}
diff --git a/src/api/api-inl.h b/src/api/api-inl.h
index 2a70d67..1f2597f 100644
--- a/src/api/api-inl.h
+++ b/src/api/api-inl.h
@@ -399,6 +399,11 @@
return handle_scope_.Escape(value);
}
+ template <typename T>
+ V8_INLINE MaybeLocal<T> EscapeMaybe(MaybeLocal<T> value) {
+ return handle_scope_.EscapeMaybe(value);
+ }
+
private:
InternalEscapableScope handle_scope_;
CallDepthScope<false> call_depth_scope_;
diff --git a/src/api/api.cc b/src/api/api.cc
index 48dcaa9..2ac3992 100644
--- a/src/api/api.cc
+++ b/src/api/api.cc
@@ -3766,7 +3766,8 @@
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Object_ToPrimitive};
i::Isolate* i_isolate = api_scope.i_isolate();
return api_scope.EscapeMaybe(
- ToMaybeLocal<Primitive>(i::Object::ToPrimitive(i_isolate, obj)));
+ Utils::ToMaybeLocal(i::Object::ToPrimitive(i_isolate, obj))
+ .As<Primitive>());
}
MaybeLocal<Numeric> Value::ToNumeric(Local<Context> context) const {
@@ -3775,7 +3776,7 @@
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Object_ToNumeric};
i::Isolate* i_isolate = api_scope.i_isolate();
return api_scope.EscapeMaybe(
- ToMaybeLocal<Numeric>(i::Object::ToNumeric(i_isolate, obj)));
+ Utils::ToMaybeLocal(i::Object::ToNumeric(i_isolate, obj)).As<Numeric>());
}
Local<Boolean> Value::ToBoolean(Isolate* v8_isolate) const {
@@ -3792,7 +3793,7 @@
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Object_ToNumber};
i::Isolate* i_isolate = api_scope.i_isolate();
return api_scope.EscapeMaybe(
- ToMaybeLocal<Number>(i::Object::ToNumber(i_isolate, obj)));
+ Utils::ToMaybeLocal(i::Object::ToNumber(i_isolate, obj)).As<Number>());
}
MaybeLocal<Integer> Value::ToInteger(Local<Context> context) const {
@@ -3801,7 +3802,7 @@
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Object_ToInteger};
i::Isolate* i_isolate = api_scope.i_isolate();
return api_scope.EscapeMaybe(
- ToMaybeLocal<Integer>(i::Object::ToInteger(i_isolate, obj)));
+ Utils::ToMaybeLocal(i::Object::ToInteger(i_isolate, obj)).As<Integer>());
}
MaybeLocal<Int32> Value::ToInt32(Local<Context> context) const {
@@ -3810,7 +3811,7 @@
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Object_ToInt32};
i::Isolate* i_isolate = api_scope.i_isolate();
return api_scope.EscapeMaybe(
- ToMaybeLocal<Int32>(i::Object::ToInt32(i_isolate, obj)));
+ Utils::ToMaybeLocal(i::Object::ToInt32(i_isolate, obj)).As<Int32>());
}
MaybeLocal<Uint32> Value::ToUint32(Local<Context> context) const {
@@ -3819,7 +3820,7 @@
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Object_ToUint32};
i::Isolate* i_isolate = api_scope.i_isolate();
return api_scope.EscapeMaybe(
- ToMaybeLocal<Uint32>(i::Object::ToUint32(i_isolate, obj)));
+ Utils::ToMaybeLocal(i::Object::ToUint32(i_isolate, obj)).As<Uint32>());
}
i::Isolate* i::IsolateFromNeverReadOnlySpaceObject(i::Address obj) {
@@ -5040,10 +5041,9 @@
i::PropertyKey lookup_key(i_isolate, key_obj);
i::LookupIterator it(i_isolate, self, lookup_key, proto,
i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
- Local<Value> result;
- if (!ToLocal<Value>(i::Object::GetProperty(&it), &result)) return {};
+ MaybeLocal<Value> result = Utils::ToMaybeLocal(i::Object::GetProperty(&it));
if (!it.IsFound()) return {};
- return api_scope.Escape(result);
+ return api_scope.EscapeMaybe(result);
}
Maybe<PropertyAttribute>
@@ -5081,10 +5081,9 @@
i::PropertyKey lookup_key(i_isolate, key_obj);
i::LookupIterator it(i_isolate, self, lookup_key, self,
i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
- Local<Value> result;
- if (!ToLocal<Value>(i::Object::GetProperty(&it), &result)) return {};
+ MaybeLocal<Value> result = Utils::ToMaybeLocal(i::Object::GetProperty(&it));
if (!it.IsFound()) return {};
- return api_scope.Escape(result);
+ return api_scope.EscapeMaybe(result);
}
Maybe<PropertyAttribute> v8::Object::GetRealNamedPropertyAttributes(
@@ -5280,7 +5279,7 @@
auto recv_obj = Utils::OpenDirectHandle(*recv);
auto args = PrepareArguments(argc, argv);
return api_scope.EscapeMaybe(
- ToMaybeLocal<Value>(i::Execution::Call(i_isolate, self, recv_obj, args)));
+ Utils::ToMaybeLocal(i::Execution::Call(i_isolate, self, recv_obj, args)));
}
MaybeLocal<Value> Object::CallAsConstructor(Local<Context> context, int argc,
@@ -5295,7 +5294,7 @@
auto self = Utils::OpenDirectHandle(this);
auto args = PrepareArguments(argc, argv);
return api_scope.EscapeMaybe(
- ToMaybeLocal<Value>(i::Execution::New(i_isolate, self, self, args)));
+ Utils::ToMaybeLocal(i::Execution::New(i_isolate, self, self, args)));
}
MaybeLocal<Function> Function::New(Local<Context> context,
@@ -5345,7 +5344,7 @@
}
auto args = PrepareArguments(argc, argv);
return api_scope.EscapeMaybe(
- ToMaybeLocal<Object>(i::Execution::New(i_isolate, self, self, args)));
+ Utils::ToMaybeLocal(i::Execution::New(i_isolate, self, self, args)));
}
MaybeLocal<v8::Value> Function::Call(v8::Isolate* isolate,
@@ -5365,7 +5364,7 @@
auto recv_obj = Utils::OpenDirectHandle(*recv);
auto args = PrepareArguments(argc, argv);
return api_scope.EscapeMaybe(
- ToMaybeLocal<Value>(i::Execution::Call(i_isolate, self, recv_obj, args)));
+ Utils::ToMaybeLocal(i::Execution::Call(i_isolate, self, recv_obj, args)));
}
MaybeLocal<v8::Value> Function::Call(Local<Context> context,
@@ -7958,7 +7957,7 @@
}
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Date_New};
i::Isolate* i_isolate = api_scope.i_isolate();
- return api_scope.EscapeMaybe(ToMaybeLocal<Value>(i::JSDate::New(
+ return api_scope.EscapeMaybe(Utils::ToMaybeLocal(i::JSDate::New(
i_isolate->date_function(), i_isolate->date_function(), time)));
}
@@ -7968,7 +7967,7 @@
auto string = Utils::OpenDirectHandle(*value);
double time = ParseDateTimeString(i_isolate, string);
- return api_scope.EscapeMaybe(ToMaybeLocal<Value>(i::JSDate::New(
+ return api_scope.EscapeMaybe(Utils::ToMaybeLocal(i::JSDate::New(
i_isolate->date_function(), i_isolate->date_function(), time)));
}
@@ -8079,9 +8078,17 @@
// mind. It fetches the 'exec' property and then calls it through JSEntry.
// Unfortunately, this is currently the only full implementation of
// RegExp.prototype.exec available in C++.
- return api_scope.EscapeMaybe(ToMaybeLocal<Object>(
- i::RegExpUtils::RegExpExec(i_isolate, regexp, subject_string,
- i_isolate->factory()->undefined_value())));
+ i::DirectHandle<i::JSAny> result;
+ if (!i::RegExpUtils::RegExpExec(i_isolate, regexp, subject_string,
+ i_isolate->factory()->undefined_value())
+ .ToHandle(&result)) {
+ return {};
+ }
+ // RegExp::Exec is documented to return Null on failure, but the return type
+ // is `MaybeLocal<Object>` which cannot legally hold `Null`, thus the unsafe
+ // `Utils::Convert<Object>`.
+ // TODO(jgruber): Fix this.
+ return api_scope.Escape(Utils::Convert<i::JSAny, Object>(result));
}
Local<v8::Array> v8::Array::New(Isolate* v8_isolate, int length) {
@@ -8599,15 +8606,13 @@
MaybeLocal<Promise::Resolver> Promise::Resolver::New(Local<Context> context) {
PrepareForExecutionScope api_scope{context, RCCId::kAPI_Promise_Resolver_New};
i::Isolate* i_isolate = api_scope.i_isolate();
- Local<Promise::Resolver> result;
- if (!ToLocal<Promise::Resolver>(i_isolate->factory()->NewJSPromise(),
- &result) ||
- // Also check if promise hooks set an exception.
- // TODO(clemensb): Should `Factory::NewJSPromise()` return a MaybeHandle
- // instead?
- i_isolate->has_exception()) {
- return {};
- }
+ Local<Promise::Resolver> result =
+ Utils::ToLocal(Cast<i::JSObject>(i_isolate->factory()->NewJSPromise()))
+ .As<Promise::Resolver>();
+ // Also check if promise hooks set an exception.
+ // TODO(clemensb): Should `Factory::NewJSPromise()` return a MaybeHandle
+ // instead?
+ if (i_isolate->has_exception()) return {};
return api_scope.Escape(result);
}
diff --git a/src/api/api.h b/src/api/api.h
index c2bfb00..5d4bbd7 100644
--- a/src/api/api.h
+++ b/src/api/api.h
@@ -95,6 +95,7 @@
V(ToLocal, Name, Name) \
V(ToLocal, String, String) \
V(ToLocal, Symbol, Symbol) \
+ V(ToLocal, JSDate, Object) \
V(ToLocal, JSRegExp, RegExp) \
V(ToLocal, JSReceiver, Object) \
V(ToLocal, JSObject, Object) \
@@ -312,22 +313,6 @@
return Utils::Convert<i::Object, T>(obj);
}
-// Convert MaybeDirectHandle to MaybeLocal w/o type inference or type checks.
-// To get type inference (translating from internal to API types), use
-// Utils::ToMaybeLocal.
-template <class T>
-inline MaybeLocal<T> ToMaybeLocal(i::MaybeDirectHandle<i::Object> maybe) {
- i::DirectHandle<i::Object> handle;
- if (maybe.ToHandle(&handle)) return Utils::Convert<i::Object, T>(handle);
- return {};
-}
-
-// Same as above, but writes into an output variable and returns bool.
-template <class T>
-inline bool ToLocal(i::MaybeDirectHandle<i::Object> maybe, Local<T>* local) {
- return ToMaybeLocal<T>(maybe).ToLocal(local);
-}
-
namespace internal {
class PersistentHandles;
diff --git a/src/debug/debug-interface.cc b/src/debug/debug-interface.cc
index a88704b..7e86aed 100644
--- a/src/debug/debug-interface.cc
+++ b/src/debug/debug-interface.cc
@@ -1212,15 +1212,12 @@
if (throw_on_side_effect) {
isolate->debug()->StartSideEffectCheckMode();
}
- Local<Value> result;
- bool has_exception = !ToLocal<Value>(
- i::Execution::Call(isolate, self, recv_obj, {arguments, args.size()}),
- &result);
+ MaybeLocal<Value> result = Utils::ToMaybeLocal(
+ i::Execution::Call(isolate, self, recv_obj, {arguments, args.size()}));
if (throw_on_side_effect) {
isolate->debug()->StopSideEffectCheckMode();
}
- if (has_exception) return {};
- return api_scope.Escape(result);
+ return api_scope.EscapeMaybe(result);
}
MaybeLocal<v8::Value> EvaluateGlobal(v8::Isolate* isolate,
@@ -1230,13 +1227,9 @@
v8::Local<v8::Context> context = Utils::ToLocal(i_isolate->native_context());
PrepareForDebugInterfaceExecutionScope api_scope(i_isolate, context);
i::REPLMode repl_mode = repl ? i::REPLMode::kYes : i::REPLMode::kNo;
- Local<Value> result;
- bool has_exception = !ToLocal<Value>(
- i::DebugEvaluate::Global(i_isolate, Utils::OpenHandle(*source), mode,
- repl_mode),
- &result);
- if (has_exception) return {};
- return api_scope.Escape(result);
+ MaybeLocal<Value> result = Utils::ToMaybeLocal(i::DebugEvaluate::Global(
+ i_isolate, Utils::OpenHandle(*source), mode, repl_mode));
+ return api_scope.EscapeMaybe(result);
}
void GlobalLexicalScopeNames(v8::Local<v8::Context> v8_context,
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 45ea5be..28ec5cb 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -19416,6 +19416,7 @@
CHECK(result0->IsArray());
v8::Local<v8::Object> result1 =
regexp->Exec(context.local(), v8_str("abd")).ToLocalChecked();
+ // TODO(jgruber): Fix the API; `v8::Null` is not a valid `v8::Object`.
CHECK(result1->IsNull());
}
}
