From 8beb98eed3bbc8645b2d10d285ae6fc038f6df09 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Mon, 7 Dec 2020 17:02:29 -0500 Subject: [PATCH] feat(neon): Update `try-catch` to match the RFC --- crates/neon-sys/native/src/neon.cc | 6 ++-- crates/neon-sys/native/src/neon.h | 4 +-- crates/neon-sys/src/lib.rs | 5 +-- src/context/internal.rs | 41 ++++++++++++------------- src/context/mod.rs | 16 +++------- test/dynamic/native/src/js/functions.rs | 8 ++--- test/napi/native/src/js/functions.rs | 8 ++--- 7 files changed, 40 insertions(+), 48 deletions(-) diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index fb3ccb8e6..a939c5a8f 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -542,10 +542,10 @@ extern "C" void Neon_EventHandler_Delete(void * thread_safe_cb) { cb->close(); } -extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, void *rust_thunk, void *cx, v8::Local *result, void **unwind_value) { +extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, void *rust_thunk, void *cx, void *ok, v8::Local *err, void **unwind_value) { Nan::TryCatch try_catch; - try_catch_control_t ctrl = glue_fn(rust_thunk, cx, result, unwind_value); + try_catch_control_t ctrl = glue_fn(rust_thunk, cx, ok, unwind_value); if (ctrl == CONTROL_PANICKED) { return CONTROL_PANICKED; @@ -560,7 +560,7 @@ extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, voi } return CONTROL_RETURNED; } else { - *result = try_catch.Exception(); + *err = try_catch.Exception(); return CONTROL_THREW; } } diff --git a/crates/neon-sys/native/src/neon.h b/crates/neon-sys/native/src/neon.h index 2542901b2..32267d8c6 100644 --- a/crates/neon-sys/native/src/neon.h +++ b/crates/neon-sys/native/src/neon.h @@ -152,13 +152,13 @@ extern "C" { // returns `CONTROL_RETURNED`. // The `unwind_value` out-parameter can be assumed to be initialized if and only if this // function returns `CONTROL_PANICKED`. - typedef try_catch_control_t (*Neon_TryCatchGlue)(void *rust_thunk, void *cx, v8::Local *result, void **unwind_value); + typedef try_catch_control_t (*Neon_TryCatchGlue)(void *rust_thunk, void *cx, void *ok, void **unwind_value); // The `result` out-parameter can be assumed to be initialized if and only if this function // returns `CONTROL_RETURNED` or `CONTROL_THREW`. // The `unwind_value` out-parameter can be assumed to be initialized if and only if this // function returns `CONTROL_PANICKED`. - try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue, void *rust_thunk, void *cx, v8::Local *result, void **unwind_value); + try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue, void *rust_thunk, void *cx, void *ok, v8::Local *err, void **unwind_value); } #endif diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index d1ee2d2df..bc3eabfd3 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -97,7 +97,7 @@ pub struct InheritedHandleScope; /// glue function returns `TryCatchControl::Panicked`. pub type TryCatchGlue = extern fn(rust_thunk: *mut c_void, cx: *mut c_void, - result: *mut Local, + result: *mut c_void, unwind_value: *mut *mut c_void) -> TryCatchControl; extern "C" { @@ -230,6 +230,7 @@ extern "C" { pub fn Neon_TryCatch_With(glue: TryCatchGlue, rust_thunk: *mut c_void, cx: *mut c_void, - result: *mut Local, + ok: *mut c_void, + err: *mut Local, unwind_value: *mut *mut c_void) -> TryCatchControl; } diff --git a/src/context/internal.rs b/src/context/internal.rs index 3788bc5f4..6829a0009 100644 --- a/src/context/internal.rs +++ b/src/context/internal.rs @@ -6,16 +6,16 @@ use std::cell::Cell; use std::mem::MaybeUninit; use std::os::raw::c_void; #[cfg(feature = "legacy-runtime")] -use std::panic::{AssertUnwindSafe, UnwindSafe, catch_unwind, resume_unwind}; +use std::panic::{AssertUnwindSafe, catch_unwind, resume_unwind}; use neon_runtime; use neon_runtime::raw; use neon_runtime::scope::Root; #[cfg(feature = "legacy-runtime")] use neon_runtime::try_catch::TryCatchControl; -use types::{JsObject, JsValue, Value}; +use types::{JsObject, JsValue}; use handle::Handle; use object::class::ClassMap; -use result::{JsResult, NeonResult}; +use result::NeonResult; use super::ModuleContext; #[cfg(feature = "legacy-runtime")] @@ -126,22 +126,24 @@ pub trait ContextInternal<'a>: Sized { fn deactivate(&self) { self.scope_metadata().active.set(false); } #[cfg(feature = "legacy-runtime")] - fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result, Handle<'a, JsValue>> - where T: Value, - F: UnwindSafe + FnOnce(&mut Self) -> JsResult<'b, T> + fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result> + where T: Sized, + F: FnOnce(&mut Self) -> NeonResult, { // A closure does not have a guaranteed layout, so we need to box it in order to pass // a pointer to it across the boundary into C++. let rust_thunk = Box::into_raw(Box::new(f)); - let mut local: MaybeUninit = MaybeUninit::zeroed(); + let mut ok: MaybeUninit = MaybeUninit::zeroed(); + let mut err: MaybeUninit = MaybeUninit::zeroed(); let mut unwind_value: MaybeUninit<*mut c_void> = MaybeUninit::zeroed(); let ctrl = unsafe { neon_runtime::try_catch::with(try_catch_glue::, rust_thunk as *mut c_void, (self as *mut Self) as *mut c_void, - local.as_mut_ptr(), + ok.as_mut_ptr() as *mut c_void, + err.as_mut_ptr(), unwind_value.as_mut_ptr()) }; @@ -152,13 +154,10 @@ pub trait ContextInternal<'a>: Sized { }; resume_unwind(unwind_value); } - TryCatchControl::Returned => { - let local = unsafe { local.assume_init() }; - Ok(Handle::new_internal(T::from_raw(self.env(), local))) - } + TryCatchControl::Returned => Ok(unsafe { ok.assume_init() }), TryCatchControl::Threw => { - let local = unsafe { local.assume_init() }; - Err(JsValue::new_internal(local)) + let err = unsafe { err.assume_init() }; + Err(JsValue::new_internal(err)) } TryCatchControl::UnexpectedErr => { panic!("try_catch: unexpected Err(Throw) when VM is not in a throwing state"); @@ -167,9 +166,9 @@ pub trait ContextInternal<'a>: Sized { } #[cfg(feature = "napi-runtime")] - fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result, Handle<'a, JsValue>> - where T: Value, - F: FnOnce(&mut Self) -> JsResult<'b, T> + fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result> + where T: Sized, + F: FnOnce(&mut Self) -> NeonResult { let result = f(self); let mut local: MaybeUninit = MaybeUninit::zeroed(); @@ -188,11 +187,11 @@ pub trait ContextInternal<'a>: Sized { #[cfg(feature = "legacy-runtime")] extern "C" fn try_catch_glue<'a, 'b: 'a, C, T, F>(rust_thunk: *mut c_void, cx: *mut c_void, - returned: *mut raw::Local, + returned: *mut c_void, unwind_value: *mut *mut c_void) -> TryCatchControl where C: ContextInternal<'a>, - T: Value, - F: UnwindSafe + FnOnce(&mut C) -> JsResult<'b, T> + T: Sized, + F: FnOnce(&mut C) -> NeonResult, { let f: F = *unsafe { Box::from_raw(rust_thunk as *mut F) }; let cx: &mut C = unsafe { std::mem::transmute(cx) }; @@ -205,7 +204,7 @@ extern "C" fn try_catch_glue<'a, 'b: 'a, C, T, F>(rust_thunk: *mut c_void, match catch_unwind(AssertUnwindSafe(|| f(cx))) { // No Rust panic, no JS exception. Ok(Ok(result)) => unsafe { - *returned = result.to_raw(); + (returned as *mut T).write(result); TryCatchControl::Returned } // No Rust panic, caught a JS exception. diff --git a/src/context/mod.rs b/src/context/mod.rs index 7e5aea144..fbb8e6fa4 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -242,18 +242,10 @@ pub trait Context<'a>: ContextInternal<'a> { result } - #[cfg(all(feature = "try-catch-api", feature = "napi-runtime"))] - fn try_catch<'b: 'a, T, F>(&mut self, f: F) -> Result, Handle<'a, JsValue>> - where T: Value, - F: FnOnce(&mut Self) -> JsResult<'b, T> - { - self.try_catch_internal(f) - } - - #[cfg(all(feature = "try-catch-api", feature = "legacy-runtime"))] - fn try_catch<'b: 'a, T, F>(&mut self, f: F) -> Result, Handle<'a, JsValue>> - where T: Value, - F: UnwindSafe + FnOnce(&mut Self) -> JsResult<'b, T> + #[cfg(feature = "try-catch-api")] + fn try_catch<'b: 'a, T, F>(&mut self, f: F) -> Result> + where T: Sized, + F: FnOnce(&mut Self) -> NeonResult { self.try_catch_internal(f) } diff --git a/test/dynamic/native/src/js/functions.rs b/test/dynamic/native/src/js/functions.rs index bc6a0ff6f..08e5f888b 100644 --- a/test/dynamic/native/src/js/functions.rs +++ b/test/dynamic/native/src/js/functions.rs @@ -103,10 +103,10 @@ pub fn compute_scoped(mut cx: FunctionContext) -> JsResult { pub fn throw_and_catch(mut cx: FunctionContext) -> JsResult { let v = cx.argument_opt(0).unwrap_or_else(|| cx.undefined().upcast()); - Ok(cx.try_catch(|cx| { - let _ = cx.throw(v)?; - Ok(cx.string("unreachable").upcast()) - }).unwrap_or_else(|err| err)) + + cx.try_catch(|cx| cx.throw(v)) + .map(|_: ()| Ok(cx.string("unreachable").upcast())) + .unwrap_or_else(|err| Ok(err)) } pub fn call_and_catch(mut cx: FunctionContext) -> JsResult { diff --git a/test/napi/native/src/js/functions.rs b/test/napi/native/src/js/functions.rs index cb32b2771..a25b39fc9 100644 --- a/test/napi/native/src/js/functions.rs +++ b/test/napi/native/src/js/functions.rs @@ -104,10 +104,10 @@ pub fn compute_scoped(mut cx: FunctionContext) -> JsResult { pub fn throw_and_catch(mut cx: FunctionContext) -> JsResult { let v = cx.argument_opt(0).unwrap_or_else(|| cx.undefined().upcast()); - Ok(cx.try_catch(|cx| { - let _ = cx.throw(v)?; - Ok(cx.string("unreachable").upcast()) - }).unwrap_or_else(|err| err)) + + cx.try_catch(|cx| cx.throw(v)) + .map(|_: ()| Ok(cx.string("unreachable").upcast())) + .unwrap_or_else(|err| Ok(err)) } pub fn call_and_catch(mut cx: FunctionContext) -> JsResult {