From 30d00f4e7210f1857625f32a915931cd00ab93da Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 5 Sep 2024 10:39:44 -0400 Subject: [PATCH 1/3] Add build_result fn --- crates/neon/src/types_impl/mod.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/neon/src/types_impl/mod.rs b/crates/neon/src/types_impl/mod.rs index 3f8677e51..1b0387ffd 100644 --- a/crates/neon/src/types_impl/mod.rs +++ b/crates/neon/src/types_impl/mod.rs @@ -63,14 +63,22 @@ pub(crate) fn build<'a, T: Value, F: FnOnce(&mut raw::Local) -> bool>( env: Env, init: F, ) -> JsResult<'a, T> { - unsafe { - let mut local: raw::Local = std::mem::zeroed(); - if init(&mut local) { - Ok(Handle::new_internal(T::from_local(env, local))) - } else { - Err(Throw::new()) + build_result(env, move || { + let mut local: raw::Local = std::ptr::null_mut(); + if !init(&mut local) { + unsafe { return Err(Throw::new()) } } - } + Ok(local) + }) +} + +pub(crate) fn build_result<'a, T: Value, E, F: FnOnce() -> Result>( + env: Env, + init: F, +) -> Result, E> { + let local = init()?; + let value = unsafe { T::from_local(env, local) }; + Ok(Handle::new_internal(value)) } impl SuperType for JsValue { From a122887abe60b79ae0fa887347ead32a06e2b756 Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 5 Sep 2024 10:54:09 -0400 Subject: [PATCH 2/3] Use build_result fn directly --- crates/neon/src/object/mod.rs | 19 ++++++++--- crates/neon/src/reflect.rs | 14 +++++--- crates/neon/src/types_impl/error.rs | 26 +++++++++------ crates/neon/src/types_impl/mod.rs | 50 ++++++++++++++++------------- 4 files changed, 67 insertions(+), 42 deletions(-) diff --git a/crates/neon/src/object/mod.rs b/crates/neon/src/object/mod.rs index 319e2d650..24393b5eb 100644 --- a/crates/neon/src/object/mod.rs +++ b/crates/neon/src/object/mod.rs @@ -37,7 +37,9 @@ use crate::{ handle::{Handle, Root}, result::{NeonResult, Throw}, sys::{self, raw}, - types::{build, function::CallOptions, utf8::Utf8, JsFunction, JsUndefined, JsValue, Value}, + types::{ + build_result, function::CallOptions, utf8::Utf8, JsFunction, JsUndefined, JsValue, Value, + }, }; #[cfg(feature = "napi-6")] @@ -161,8 +163,12 @@ pub trait Object: Value { cx: &mut C, key: K, ) -> NeonResult> { - build(cx.env(), |out| unsafe { - key.get_from(cx, out, self.to_local()) + let env = cx.env(); + build_result(env, move || unsafe { + let mut out: raw::Local = std::ptr::null_mut(); + key.get_from(cx, &mut out, self.to_local()) + .then_some(out) + .ok_or(Throw::new()) }) } @@ -183,8 +189,11 @@ pub trait Object: Value { fn get_own_property_names<'a, C: Context<'a>>(&self, cx: &mut C) -> JsResult<'a, JsArray> { let env = cx.env(); - build(cx.env(), |out| unsafe { - sys::object::get_own_property_names(out, env.to_raw(), self.to_local()) + build_result(env, move || unsafe { + let mut out: raw::Local = std::ptr::null_mut(); + sys::object::get_own_property_names(&mut out, env.to_raw(), self.to_local()) + .then_some(out) + .ok_or(Throw::new()) }) } diff --git a/crates/neon/src/reflect.rs b/crates/neon/src/reflect.rs index 6821b0139..bb08b7285 100644 --- a/crates/neon/src/reflect.rs +++ b/crates/neon/src/reflect.rs @@ -3,16 +3,20 @@ use crate::{ context::Context, handle::Handle, - result::JsResult, - types::{build, private::ValueInternal, JsString, JsValue}, + result::{JsResult, Throw}, + sys::raw, + types::{build_result, private::ValueInternal, JsString, JsValue}, }; pub fn eval<'a, 'b, C: Context<'a>>( cx: &mut C, script: Handle<'b, JsString>, ) -> JsResult<'a, JsValue> { - let env = cx.env().to_raw(); - build(cx.env(), |out| unsafe { - crate::sys::string::run_script(out, env, script.to_local()) + let env = cx.env(); + build_result(env, move || unsafe { + let mut out: raw::Local = std::ptr::null_mut(); + crate::sys::string::run_script(&mut out, env.to_raw(), script.to_local()) + .then_some(out) + .ok_or(Throw::new()) }) } diff --git a/crates/neon/src/types_impl/error.rs b/crates/neon/src/types_impl/error.rs index 98fc404b8..fde527a8b 100644 --- a/crates/neon/src/types_impl/error.rs +++ b/crates/neon/src/types_impl/error.rs @@ -8,7 +8,7 @@ use crate::{ object::Object, result::{NeonResult, Throw}, sys::{self, raw}, - types::{build, private::ValueInternal, utf8::Utf8, Value}, + types::{build_result, private::ValueInternal, utf8::Utf8, Value}, }; /// The type of JavaScript @@ -77,10 +77,12 @@ impl JsError { cx: &mut C, msg: S, ) -> NeonResult> { + let env = cx.env(); let msg = cx.string(msg.as_ref()); - build(cx.env(), |out| unsafe { - sys::error::new_error(cx.env().to_raw(), out, msg.to_local()); - true + build_result(env, || unsafe { + let mut out = std::ptr::null_mut(); + sys::error::new_error(env.to_raw(), &mut out, msg.to_local()); + Ok(out) }) } @@ -91,10 +93,12 @@ impl JsError { cx: &mut C, msg: S, ) -> NeonResult> { + let env = cx.env(); let msg = cx.string(msg.as_ref()); - build(cx.env(), |out| unsafe { - sys::error::new_type_error(cx.env().to_raw(), out, msg.to_local()); - true + build_result(env, || unsafe { + let mut out = std::ptr::null_mut(); + sys::error::new_type_error(env.to_raw(), &mut out, msg.to_local()); + Ok(out) }) } @@ -105,10 +109,12 @@ impl JsError { cx: &mut C, msg: S, ) -> NeonResult> { + let env = cx.env(); let msg = cx.string(msg.as_ref()); - build(cx.env(), |out| unsafe { - sys::error::new_range_error(cx.env().to_raw(), out, msg.to_local()); - true + build_result(env, move || unsafe { + let mut out: raw::Local = std::ptr::null_mut(); + sys::error::new_range_error(env.to_raw(), &mut out, msg.to_local()); + Ok(out) }) } } diff --git a/crates/neon/src/types_impl/mod.rs b/crates/neon/src/types_impl/mod.rs index 1b0387ffd..a57429241 100644 --- a/crates/neon/src/types_impl/mod.rs +++ b/crates/neon/src/types_impl/mod.rs @@ -59,19 +59,6 @@ pub use self::promise::JsFuture; // This should be considered deprecated and will be removed: // https://github.com/neon-bindings/neon/issues/983 -pub(crate) fn build<'a, T: Value, F: FnOnce(&mut raw::Local) -> bool>( - env: Env, - init: F, -) -> JsResult<'a, T> { - build_result(env, move || { - let mut local: raw::Local = std::ptr::null_mut(); - if !init(&mut local) { - unsafe { return Err(Throw::new()) } - } - Ok(local) - }) -} - pub(crate) fn build_result<'a, T: Value, E, F: FnOnce() -> Result>( env: Env, init: F, @@ -97,8 +84,11 @@ impl SuperType for JsObject { pub trait Value: ValueInternal { fn to_string<'cx, C: Context<'cx>>(&self, cx: &mut C) -> JsResult<'cx, JsString> { let env = cx.env(); - build(env, |out| unsafe { - sys::convert::to_string(out, env.to_raw(), self.to_local()) + build_result(env, || unsafe { + let mut out = std::ptr::null_mut(); + sys::convert::to_string(&mut out, env.to_raw(), self.to_local()) + .then_some(out) + .ok_or(Throw::new()) }) } @@ -1177,9 +1167,20 @@ impl JsFunction { AS: AsRef<[Handle<'b, JsValue>]>, { let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?; - let env = cx.env().to_raw(); - build(cx.env(), |out| unsafe { - sys::fun::call(out, env, self.to_local(), this.to_local(), argc, argv) + let env = cx.env(); + build_result(env, move || unsafe { + let mut out: raw::Local = std::ptr::null_mut(); + + sys::fun::call( + &mut out, + env.to_raw(), + self.to_local(), + this.to_local(), + argc, + argv, + ) + .then_some(out) + .ok_or(Throw::new()) }) } @@ -1212,10 +1213,15 @@ impl JsFunction { AS: AsRef<[Handle<'b, JsValue>]>, { let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?; - let env = cx.env().to_raw(); - build(cx.env(), |out| unsafe { - sys::fun::construct(out, env, self.to_local(), argc, argv) - }) + let env = cx.env(); + { + build_result(env, move || unsafe { + let mut out: raw::Local = std::ptr::null_mut(); + sys::fun::construct(&mut out, env.to_raw(), self.to_local(), argc, argv) + .then_some(out) + .ok_or(Throw::new()) + }) + } } } From 245963903c3617ad8d1f5bc898957ca4929112f5 Mon Sep 17 00:00:00 2001 From: Alex Konradi Date: Thu, 5 Sep 2024 11:05:49 -0400 Subject: [PATCH 3/3] Rename build_result as build --- crates/neon/src/object/mod.rs | 9 +++------ crates/neon/src/reflect.rs | 4 ++-- crates/neon/src/types_impl/error.rs | 16 +++++++++------- crates/neon/src/types_impl/mod.rs | 8 ++++---- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/neon/src/object/mod.rs b/crates/neon/src/object/mod.rs index 24393b5eb..093362870 100644 --- a/crates/neon/src/object/mod.rs +++ b/crates/neon/src/object/mod.rs @@ -37,9 +37,7 @@ use crate::{ handle::{Handle, Root}, result::{NeonResult, Throw}, sys::{self, raw}, - types::{ - build_result, function::CallOptions, utf8::Utf8, JsFunction, JsUndefined, JsValue, Value, - }, + types::{build, function::CallOptions, utf8::Utf8, JsFunction, JsUndefined, JsValue, Value}, }; #[cfg(feature = "napi-6")] @@ -163,8 +161,7 @@ pub trait Object: Value { cx: &mut C, key: K, ) -> NeonResult> { - let env = cx.env(); - build_result(env, move || unsafe { + build(cx.env(), move || unsafe { let mut out: raw::Local = std::ptr::null_mut(); key.get_from(cx, &mut out, self.to_local()) .then_some(out) @@ -189,7 +186,7 @@ pub trait Object: Value { fn get_own_property_names<'a, C: Context<'a>>(&self, cx: &mut C) -> JsResult<'a, JsArray> { let env = cx.env(); - build_result(env, move || unsafe { + build(env, move || unsafe { let mut out: raw::Local = std::ptr::null_mut(); sys::object::get_own_property_names(&mut out, env.to_raw(), self.to_local()) .then_some(out) diff --git a/crates/neon/src/reflect.rs b/crates/neon/src/reflect.rs index bb08b7285..ca09832c2 100644 --- a/crates/neon/src/reflect.rs +++ b/crates/neon/src/reflect.rs @@ -5,7 +5,7 @@ use crate::{ handle::Handle, result::{JsResult, Throw}, sys::raw, - types::{build_result, private::ValueInternal, JsString, JsValue}, + types::{build, private::ValueInternal, JsString, JsValue}, }; pub fn eval<'a, 'b, C: Context<'a>>( @@ -13,7 +13,7 @@ pub fn eval<'a, 'b, C: Context<'a>>( script: Handle<'b, JsString>, ) -> JsResult<'a, JsValue> { let env = cx.env(); - build_result(env, move || unsafe { + build(env, move || unsafe { let mut out: raw::Local = std::ptr::null_mut(); crate::sys::string::run_script(&mut out, env.to_raw(), script.to_local()) .then_some(out) diff --git a/crates/neon/src/types_impl/error.rs b/crates/neon/src/types_impl/error.rs index fde527a8b..614ef38dc 100644 --- a/crates/neon/src/types_impl/error.rs +++ b/crates/neon/src/types_impl/error.rs @@ -8,7 +8,7 @@ use crate::{ object::Object, result::{NeonResult, Throw}, sys::{self, raw}, - types::{build_result, private::ValueInternal, utf8::Utf8, Value}, + types::{build, private::ValueInternal, utf8::Utf8, Value}, }; /// The type of JavaScript @@ -79,11 +79,13 @@ impl JsError { ) -> NeonResult> { let env = cx.env(); let msg = cx.string(msg.as_ref()); - build_result(env, || unsafe { - let mut out = std::ptr::null_mut(); + + let mut out = std::ptr::null_mut(); + let value = unsafe { sys::error::new_error(env.to_raw(), &mut out, msg.to_local()); - Ok(out) - }) + Self::from_local(env, out) + }; + Ok(Handle::new_internal(value)) } /// Creates an instance of the [`TypeError`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/TypeError) class. @@ -95,7 +97,7 @@ impl JsError { ) -> NeonResult> { let env = cx.env(); let msg = cx.string(msg.as_ref()); - build_result(env, || unsafe { + build(env, || unsafe { let mut out = std::ptr::null_mut(); sys::error::new_type_error(env.to_raw(), &mut out, msg.to_local()); Ok(out) @@ -111,7 +113,7 @@ impl JsError { ) -> NeonResult> { let env = cx.env(); let msg = cx.string(msg.as_ref()); - build_result(env, move || unsafe { + build(env, move || unsafe { let mut out: raw::Local = std::ptr::null_mut(); sys::error::new_range_error(env.to_raw(), &mut out, msg.to_local()); Ok(out) diff --git a/crates/neon/src/types_impl/mod.rs b/crates/neon/src/types_impl/mod.rs index a57429241..226ce051d 100644 --- a/crates/neon/src/types_impl/mod.rs +++ b/crates/neon/src/types_impl/mod.rs @@ -59,7 +59,7 @@ pub use self::promise::JsFuture; // This should be considered deprecated and will be removed: // https://github.com/neon-bindings/neon/issues/983 -pub(crate) fn build_result<'a, T: Value, E, F: FnOnce() -> Result>( +pub(crate) fn build<'a, T: Value, E, F: FnOnce() -> Result>( env: Env, init: F, ) -> Result, E> { @@ -84,7 +84,7 @@ impl SuperType for JsObject { pub trait Value: ValueInternal { fn to_string<'cx, C: Context<'cx>>(&self, cx: &mut C) -> JsResult<'cx, JsString> { let env = cx.env(); - build_result(env, || unsafe { + build(env, || unsafe { let mut out = std::ptr::null_mut(); sys::convert::to_string(&mut out, env.to_raw(), self.to_local()) .then_some(out) @@ -1168,7 +1168,7 @@ impl JsFunction { { let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?; let env = cx.env(); - build_result(env, move || unsafe { + build(env, move || unsafe { let mut out: raw::Local = std::ptr::null_mut(); sys::fun::call( @@ -1215,7 +1215,7 @@ impl JsFunction { let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?; let env = cx.env(); { - build_result(env, move || unsafe { + build(env, move || unsafe { let mut out: raw::Local = std::ptr::null_mut(); sys::fun::construct(&mut out, env.to_raw(), self.to_local(), argc, argv) .then_some(out)