Fix remaining Local::from_raw() misuse, and correct some lifetimes (#388)

src/array_buffer_view.rs

@@ -24,7 +24,7 @@ impl ArrayBufferView {
   /// Returns underlying ArrayBuffer.
   pub fn buffer<'sc>(
     &self,
-    scope: &'_ mut impl ToLocal<'sc>,
+    scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, ArrayBuffer>> {
     unsafe { scope.to_local(v8__ArrayBufferView__Buffer(self)) }
   }

src/isolate.rs

@@ -320,14 +320,18 @@ impl Isolate {
   /// exception has been scheduled it is illegal to invoke any JavaScript
   /// operation; the caller must return immediately and only after the exception
   /// has been handled does it become legal to invoke JavaScript operations.
-  pub fn throw_exception<'sc>(
+  ///
+  /// This function always returns the `undefined` value.
+  pub fn throw_exception(
     &mut self,
     exception: Local<Value>,
-  ) -> Local<'sc, Value> {
+  ) -> Local<'_, Value> {
-    unsafe {
+    let result = unsafe {
-      let ptr = v8__Isolate__ThrowException(self, &*exception);
+      Local::from_raw(v8__Isolate__ThrowException(self, &*exception))
-      Local::from_raw(ptr).unwrap()
     }
+    .unwrap();
+    debug_assert!(result.is_undefined());
+    result
   }
 
   /// Runs the default MicrotaskQueue until it gets empty.

src/json.rs

@@ -3,6 +3,7 @@
 use crate::Context;
 use crate::Local;
 use crate::String;
+use crate::ToLocal;
 use crate::Value;
 
 extern "C" {
@@ -19,17 +20,19 @@ extern "C" {
 /// Tries to parse the string `json_string` and returns it as value if
 /// successful.
 pub fn parse<'sc>(
-  context: Local<'sc, Context>,
+  scope: &mut impl ToLocal<'sc>,
-  json_string: Local<'sc, String>,
+  context: Local<'_, Context>,
+  json_string: Local<'_, String>,
 ) -> Option<Local<'sc, Value>> {
-  unsafe { Local::from_raw(v8__JSON__Parse(&*context, &*json_string)) }
+  unsafe { scope.to_local(v8__JSON__Parse(&*context, &*json_string)) }
 }
 
 /// Tries to stringify the JSON-serializable object `json_object` and returns
 /// it as string if successful.
 pub fn stringify<'sc>(
+  scope: &mut impl ToLocal<'sc>,
   context: Local<'sc, Context>,
   json_object: Local<'sc, Value>,
 ) -> Option<Local<'sc, String>> {
-  unsafe { Local::from_raw(v8__JSON__Stringify(&*context, &*json_object)) }
+  unsafe { scope.to_local(v8__JSON__Stringify(&*context, &*json_object)) }
 }

src/module.rs

@@ -32,7 +32,6 @@ use crate::Value;
 ///      Some(resolved_module)
 ///   }
 /// ```
-///
 
 // System V AMD64 ABI: Local<Module> returned in a register.
 #[cfg(not(target_os = "windows"))]
@@ -147,7 +146,9 @@ impl Module {
 
   /// For a module in kErrored status, this returns the corresponding exception.
   pub fn get_exception(&self) -> Local<Value> {
-    unsafe { Local::from_raw(v8__Module__GetException(self)).unwrap() }
+    // Note: the returned value is not actually stored in a HandleScope,
+    // therefore we don't need a scope object here.
+    unsafe { Local::from_raw(v8__Module__GetException(self)) }.unwrap()
   }
 
   /// Returns the number of modules requested by this module.
@@ -160,6 +161,8 @@ impl Module {
   /// Returns the ith module specifier in this module.
   /// i must be < self.get_module_requests_length() and >= 0.
   pub fn get_module_request(&self, i: usize) -> Local<String> {
+    // Note: the returned value is not actually stored in a HandleScope,
+    // therefore we don't need a scope object here.
     unsafe {
       Local::from_raw(v8__Module__GetModuleRequest(self, i.try_into().unwrap()))
     }
@@ -189,6 +192,8 @@ impl Module {
   ///
   /// The module's status must be at least kInstantiated.
   pub fn get_module_namespace(&self) -> Local<Value> {
+    // Note: the returned value is not actually stored in a HandleScope,
+    // therefore we don't need a scope object here.
     unsafe { Local::from_raw(v8__Module__GetModuleNamespace(self)).unwrap() }
   }
 

src/promise.rs

@@ -91,10 +91,11 @@ impl Promise {
   /// See `Self::then2`.
   pub fn catch<'sc>(
     &self,
-    context: Local<'sc, Context>,
+    scope: &mut impl ToLocal<'sc>,
-    handler: Local<'sc, Function>,
+    context: Local<Context>,
+    handler: Local<Function>,
   ) -> Option<Local<'sc, Promise>> {
-    unsafe { Local::from_raw(v8__Promise__Catch(&*self, &*context, &*handler)) }
+    unsafe { scope.to_local(v8__Promise__Catch(&*self, &*context, &*handler)) }
   }
 
   /// Register a resolution handler with a promise.
@@ -102,10 +103,11 @@ impl Promise {
   /// See `Self::then2`.
   pub fn then<'sc>(
     &self,
-    context: Local<'sc, Context>,
+    scope: &mut impl ToLocal<'sc>,
-    handler: Local<'sc, Function>,
+    context: Local<Context>,
+    handler: Local<Function>,
   ) -> Option<Local<'sc, Promise>> {
-    unsafe { Local::from_raw(v8__Promise__Then(&*self, &*context, &*handler)) }
+    unsafe { scope.to_local(v8__Promise__Then(&*self, &*context, &*handler)) }
   }
 
   /// Register a resolution/rejection handler with a promise.
@@ -114,12 +116,13 @@ impl Promise {
   /// invoked at the end of turn.
   pub fn then2<'sc>(
     &self,
-    context: Local<'sc, Context>,
+    scope: &mut impl ToLocal<'sc>,
-    on_fulfilled: Local<'sc, Function>,
+    context: Local<Context>,
-    on_rejected: Local<'sc, Function>,
+    on_fulfilled: Local<Function>,
+    on_rejected: Local<Function>,
   ) -> Option<Local<'sc, Promise>> {
     unsafe {
-      Local::from_raw(v8__Promise__Then2(
+      scope.to_local(v8__Promise__Then2(
         &*self,
         &*context,
         &*on_fulfilled,
@@ -182,9 +185,8 @@ pub struct PromiseRejectMessage<'msg>([usize; 3], PhantomData<&'msg ()>);
 
 impl<'msg> PromiseRejectMessage<'msg> {
   pub fn get_promise(&self) -> Local<'msg, Promise> {
-    unsafe {
+    unsafe { Local::from_raw(v8__PromiseRejectMessage__GetPromise(self)) }
-      Local::from_raw(v8__PromiseRejectMessage__GetPromise(self)).unwrap()
+      .unwrap()
-    }
   }
 
   pub fn get_event(&self) -> PromiseRejectEvent {
@@ -192,8 +194,7 @@ impl<'msg> PromiseRejectMessage<'msg> {
   }
 
   pub fn get_value(&self) -> Local<'msg, Value> {
-    unsafe {
+    unsafe { Local::from_raw(v8__PromiseRejectMessage__GetValue(self)) }
-      Local::from_raw(v8__PromiseRejectMessage__GetValue(self)).unwrap()
+      .unwrap()
-    }
   }
 }

src/scope_traits.rs

@@ -156,12 +156,16 @@ pub trait ToLocal<'s>: InIsolate {
   }
 
   fn get_current_context(&mut self) -> Option<Local<'s, Context>> {
-    unsafe { Local::from_raw(v8__Isolate__GetCurrentContext(self.isolate())) }
+    unsafe {
+      let ptr = v8__Isolate__GetCurrentContext(self.isolate());
+      self.to_local(ptr)
+    }
   }
 
   fn get_entered_or_microtask_context(&mut self) -> Option<Local<'s, Context>> {
     unsafe {
-      Local::from_raw(v8__Isolate__GetEnteredOrMicrotaskContext(self.isolate()))
+      let ptr = v8__Isolate__GetEnteredOrMicrotaskContext(self.isolate());
+      self.to_local(ptr)
     }
   }
 }

src/script_compiler.rs

@@ -1,12 +1,12 @@
 // Copyright 2019-2020 the Deno authors. All rights reserved. MIT license.
-//! For compiling scripts.
+use std::mem::MaybeUninit;
-use crate::InIsolate;
+
 use crate::Isolate;
 use crate::Local;
 use crate::Module;
 use crate::ScriptOrigin;
 use crate::String;
-use std::mem::MaybeUninit;
+use crate::ToLocal;
 
 extern "C" {
   fn v8__ScriptCompiler__Source__CONSTRUCT(
@@ -77,10 +77,10 @@ pub enum NoCacheReason {
 ///
 /// Corresponds to the ParseModule abstract operation in the ECMAScript
 /// specification.
-pub fn compile_module<'a>(
+pub fn compile_module<'sc>(
-  scope: &mut impl InIsolate,
+  scope: &mut impl ToLocal<'sc>,
   source: Source,
-) -> Option<Local<'a, Module>> {
+) -> Option<Local<'sc, Module>> {
   compile_module2(
     scope,
     source,
@@ -90,18 +90,19 @@ pub fn compile_module<'a>(
 }
 
 /// Same as compile_module with more options.
-pub fn compile_module2<'a>(
+pub fn compile_module2<'sc>(
-  scope: &mut impl InIsolate,
+  scope: &mut impl ToLocal<'sc>,
   mut source: Source,
   options: CompileOptions,
   no_cache_reason: NoCacheReason,
-) -> Option<Local<'a, Module>> {
+) -> Option<Local<'sc, Module>> {
   unsafe {
-    Local::from_raw(v8__ScriptCompiler__CompileModule(
+    let ptr = v8__ScriptCompiler__CompileModule(
       scope.isolate(),
       &mut source,
       options,
       no_cache_reason,
-    ))
+    );
+    scope.to_local(ptr)
   }
 }

src/script_or_module.rs

@@ -17,7 +17,7 @@ extern "C" {
 impl ScriptOrModule {
   /// The name that was passed by the embedder as ResourceName to the
   /// ScriptOrigin. This can be either a v8::String or v8::Undefined.
-  pub fn get_resource_name(&self) -> Local<'_, Value> {
+  pub fn get_resource_name(&self) -> Local<Value> {
     unsafe {
       let ptr = v8__ScriptOrModule__GetResourceName(self);
       Local::from_raw(ptr).unwrap()
@@ -26,7 +26,7 @@ impl ScriptOrModule {
 
   /// The options that were passed by the embedder as HostDefinedOptions to the
   /// ScriptOrigin.
-  pub fn get_host_defined_options(&self) -> Local<'_, PrimitiveArray> {
+  pub fn get_host_defined_options(&self) -> Local<PrimitiveArray> {
     unsafe {
       let ptr = v8__ScriptOrModule__GetHostDefinedOptions(self);
       Local::from_raw(ptr).unwrap()

src/shared_array_buffer.rs

@@ -50,10 +50,11 @@ impl SharedArrayBuffer {
     byte_length: usize,
   ) -> Option<Local<'sc, SharedArrayBuffer>> {
     unsafe {
-      Local::from_raw(v8__SharedArrayBuffer__New__with_byte_length(
+      let ptr = v8__SharedArrayBuffer__New__with_byte_length(
         scope.isolate(),
         byte_length,
-      ))
+      );
+      scope.to_local(ptr)
     }
   }
 

src/try_catch.rs

@@ -161,8 +161,15 @@ impl<'tc> TryCatch<'tc> {
   /// it is illegal to execute any JavaScript operations after calling
   /// ReThrow; the caller must return immediately to where the exception
   /// is caught.
-  pub fn rethrow<'a>(&'_ mut self) -> Option<Local<'a, Value>> {
+  ///
-    unsafe { Local::from_raw(v8__TryCatch__ReThrow(&mut self.0)) }
+  /// This function returns the `undefined` value when successful, or `None` if
+  /// no exception was caught and therefore there was nothing to rethrow.
+  pub fn rethrow(&mut self) -> Option<Local<'_, Value>> {
+    let result = unsafe { Local::from_raw(v8__TryCatch__ReThrow(&mut self.0)) };
+    if let Some(value) = result {
+      debug_assert!(value.is_undefined())
+    }
+    result
   }
 
   /// Returns true if verbosity is enabled.

src/uint8_array.rs

@@ -1,7 +1,7 @@
-use std::ops::DerefMut;
+// Copyright 2019-2020 the Deno authors. All rights reserved. MIT license.
-
 use crate::ArrayBuffer;
 use crate::Local;
+use crate::ToLocal;
 use crate::Uint8Array;
 
 extern "C" {
@@ -14,12 +14,11 @@ extern "C" {
 
 impl Uint8Array {
   pub fn new<'sc>(
-    mut buf: Local<ArrayBuffer>,
+    scope: &mut impl ToLocal<'sc>,
+    buf: Local<ArrayBuffer>,
     byte_offset: usize,
     length: usize,
   ) -> Option<Local<'sc, Uint8Array>> {
-    unsafe {
+    unsafe { scope.to_local(v8__Uint8Array__New(&*buf, byte_offset, length)) }
-      Local::from_raw(v8__Uint8Array__New(buf.deref_mut(), byte_offset, length))
-    }
   }
 }

src/value.rs

@@ -424,7 +424,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, BigInt>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToBigInt(self, &*context))
+      scope.to_local(v8__Value__ToBigInt(self, &*context))
     })
   }
 
@@ -433,7 +433,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, Number>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToNumber(self, &*context))
+      scope.to_local(v8__Value__ToNumber(self, &*context))
     })
   }
 
@@ -442,7 +442,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, String>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToString(self, &*context))
+      scope.to_local(v8__Value__ToString(self, &*context))
     })
   }
 
@@ -451,7 +451,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, String>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToDetailString(self, &*context))
+      scope.to_local(v8__Value__ToDetailString(self, &*context))
     })
   }
 
@@ -460,7 +460,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, Object>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToObject(self, &*context))
+      scope.to_local(v8__Value__ToObject(self, &*context))
     })
   }
 
@@ -469,7 +469,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, Integer>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToInteger(self, &*context))
+      scope.to_local(v8__Value__ToInteger(self, &*context))
     })
   }
 
@@ -478,7 +478,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, Uint32>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToUint32(self, &*context))
+      scope.to_local(v8__Value__ToUint32(self, &*context))
     })
   }
 
@@ -487,7 +487,7 @@ impl Value {
     scope: &mut impl ToLocal<'sc>,
   ) -> Option<Local<'sc, Int32>> {
     scope.get_current_context().and_then(|context| unsafe {
-      Local::from_raw(v8__Value__ToInt32(self, &*context))
+      scope.to_local(v8__Value__ToInt32(self, &*context))
     })
   }
 

tests/test_api.rs

@@ -1070,10 +1070,10 @@ fn json() {
     let mut cs = v8::ContextScope::new(scope, context);
     let scope = cs.enter();
     let json_string = v8_str(scope, "{\"a\": 1, \"b\": 2}");
-    let maybe_value = v8::json::parse(context, json_string);
+    let maybe_value = v8::json::parse(scope, context, json_string);
     assert!(maybe_value.is_some());
     let value = maybe_value.unwrap();
-    let maybe_stringified = v8::json::stringify(context, value);
+    let maybe_stringified = v8::json::stringify(scope, context, value);
     assert!(maybe_stringified.is_some());
     let stringified = maybe_stringified.unwrap();
     let rust_str = stringified.to_rust_string_lossy(scope);
@@ -1973,7 +1973,7 @@ fn uint8_array() {
     let maybe_ab = result.buffer(scope);
     assert!(maybe_ab.is_some());
     let ab = maybe_ab.unwrap();
-    let uint8_array = v8::Uint8Array::new(ab, 0, 0);
+    let uint8_array = v8::Uint8Array::new(scope, ab, 0, 0);
     assert!(uint8_array.is_some());
   }
 }