Merge pull request #825 from neon-bindings/kv/simplify-call-construct

Simplify `call` and `construct` to only take a `AsRef<Handle<JsValue>>` that can be directly passed to args
neon-bindings · Nov 17, 2021 · 4581958 · 4581958
2 parents d63f35e + 57b1101
commit 4581958
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 33 deletions.
diff --git a/crates/neon-runtime/src/napi/fun.rs b/crates/neon-runtime/src/napi/fun.rs
@@ -32,7 +32,7 @@ pub unsafe fn call(
     fun: Local,
     this: Local,
     argc: i32,
-    argv: *mut c_void,
+    argv: *const c_void,
 ) -> bool {
     let status = napi::call_function(
         env,
@@ -51,7 +51,7 @@ pub unsafe fn construct(
     env: Env,
     fun: Local,
     argc: i32,
-    argv: *mut c_void,
+    argv: *const c_void,
 ) -> bool {
     let status = napi::new_instance(env, fun, argc as usize, argv as *const _, out as *mut _);
 

diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs
@@ -206,14 +206,14 @@ extern "C" {
         fun: Local,
         this: Local,
         argc: i32,
-        argv: *mut c_void,
+        argv: *const c_void,
     ) -> bool;
     pub fn Neon_Fun_Construct(
         out: &mut Local,
         isolate: Isolate,
         fun: Local,
         argc: i32,
-        argv: *mut c_void,
+        argv: *const c_void,
     ) -> bool;
 
     pub fn Neon_Mem_SameHandle(h1: Local, h2: Local) -> bool;

diff --git a/src/event/event_handler.rs b/src/event/event_handler.rs
@@ -47,7 +47,11 @@ impl EventHandler {
         F: Send + 'static,
     {
         self.schedule_with(move |cx, this, callback| {
-            let args = arg_cb(cx);
+            let args = arg_cb(cx)
+                .into_iter()
+                .map(|v| v.upcast())
+                .collect::<smallvec::SmallVec<[_; 4]>>();
+
             let _result = callback.call(cx, this, args);
         })
     }

diff --git a/src/object/class/mod.rs b/src/object/class/mod.rs
@@ -109,6 +109,11 @@ pub trait Class: Managed + Any {
         AS: IntoIterator<Item = Handle<'b, A>>,
     {
         let constructor = Self::constructor(cx)?;
+        let args = args
+            .into_iter()
+            .map(|v| v.upcast())
+            .collect::<smallvec::SmallVec<[_; 4]>>();
+
         constructor.construct(cx, args)
     }
 

diff --git a/src/types/mod.rs b/src/types/mod.rs
@@ -100,7 +100,6 @@ use crate::result::{JsResult, JsResultExt, NeonResult, Throw};
 use crate::types::internal::Callback;
 use neon_runtime;
 use neon_runtime::raw;
-use smallvec::SmallVec;
 use std::fmt;
 use std::fmt::Debug;
 use std::marker::PhantomData;
@@ -694,19 +693,19 @@ impl<T: Object> Object for JsFunction<T> {}
 // Maximum number of function arguments in V8.
 const V8_ARGC_LIMIT: usize = 65535;
 
-unsafe fn prepare_call<'a, 'b, C: Context<'a>, A>(
+unsafe fn prepare_call<'a, 'b, C: Context<'a>>(
     cx: &mut C,
-    args: &mut [Handle<'b, A>],
-) -> NeonResult<(i32, *mut c_void)>
-where
-    A: Value + 'b,
-{
-    let argv = args.as_mut_ptr();
+    args: &[Handle<'b, JsValue>],
+) -> NeonResult<(i32, *const c_void)> {
+    // Note: This cast is only save because `Handle<'_, JsValue>` is
+    // guaranteed to have the same layout as a pointer because `Handle`
+    // and `JsValue` are both `repr(C)` newtypes.
+    let argv = args.as_ptr().cast();
     let argc = args.len();
     if argc > V8_ARGC_LIMIT {
         return cx.throw_range_error("too many arguments");
     }
-    Ok((argc as i32, argv as *mut c_void))
+    Ok((argc as i32, argv))
 }
 
 impl JsFunction {
@@ -729,32 +728,28 @@ impl JsFunction {
 }
 
 impl<CL: Object> JsFunction<CL> {
-    pub fn call<'a, 'b, C: Context<'a>, T, A, AS>(
+    pub fn call<'a, 'b, C: Context<'a>, T, AS>(
         self,
         cx: &mut C,
         this: Handle<'b, T>,
         args: AS,
     ) -> JsResult<'a, JsValue>
     where
         T: Value,
-        A: Value + 'b,
-        AS: IntoIterator<Item = Handle<'b, A>>,
+        AS: AsRef<[Handle<'b, JsValue>]>,
     {
-        let mut args = args.into_iter().collect::<SmallVec<[_; 8]>>();
-        let (argc, argv) = unsafe { prepare_call(cx, &mut args) }?;
+        let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?;
         let env = cx.env().to_raw();
         build(cx.env(), |out| unsafe {
             neon_runtime::fun::call(out, env, self.to_raw(), this.to_raw(), argc, argv)
         })
     }
 
-    pub fn construct<'a, 'b, C: Context<'a>, A, AS>(self, cx: &mut C, args: AS) -> JsResult<'a, CL>
+    pub fn construct<'a, 'b, C: Context<'a>, AS>(self, cx: &mut C, args: AS) -> JsResult<'a, CL>
     where
-        A: Value + 'b,
-        AS: IntoIterator<Item = Handle<'b, A>>,
+        AS: AsRef<[Handle<'b, JsValue>]>,
     {
-        let mut args = args.into_iter().collect::<SmallVec<[_; 8]>>();
-        let (argc, argv) = unsafe { prepare_call(cx, &mut args) }?;
+        let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?;
         let env = cx.env().to_raw();
         build(cx.env(), |out| unsafe {
             neon_runtime::fun::construct(out, env, self.to_raw(), argc, argv)

diff --git a/test/dynamic/native/src/js/functions.rs b/test/dynamic/native/src/js/functions.rs
@@ -13,7 +13,7 @@ pub fn return_js_function(mut cx: FunctionContext) -> JsResult<JsFunction> {
 
 pub fn call_js_function(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let f = cx.argument::<JsFunction>(0)?;
-    let args: Vec<Handle<JsNumber>> = vec![cx.number(16.0)];
+    let args = [cx.number(16.0).upcast()];
     let null = cx.null();
     f.call(&mut cx, null, args)?
         .downcast::<JsNumber>()
@@ -23,7 +23,7 @@ pub fn call_js_function(mut cx: FunctionContext) -> JsResult<JsNumber> {
 pub fn construct_js_function(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let f = cx.argument::<JsFunction>(0)?;
     let zero = cx.number(0.0);
-    let o = f.construct(&mut cx, vec![zero])?;
+    let o = f.construct(&mut cx, [zero.upcast()])?;
     let get_utc_full_year_method = o
         .get(&mut cx, "getUTCFullYear")?
         .downcast::<JsFunction>()

diff --git a/test/napi/src/js/functions.rs b/test/napi/src/js/functions.rs
@@ -12,7 +12,7 @@ pub fn return_js_function(mut cx: FunctionContext) -> JsResult<JsFunction> {
 
 pub fn call_js_function(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let f = cx.argument::<JsFunction>(0)?;
-    let args: Vec<Handle<JsNumber>> = vec![cx.number(16.0)];
+    let args = [cx.number(16.0).upcast()];
     let null = cx.null();
     f.call(&mut cx, null, args)?
         .downcast::<JsNumber, _>(&mut cx)
@@ -22,7 +22,7 @@ pub fn call_js_function(mut cx: FunctionContext) -> JsResult<JsNumber> {
 pub fn construct_js_function(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let f = cx.argument::<JsFunction>(0)?;
     let zero = cx.number(0.0);
-    let o = f.construct(&mut cx, vec![zero])?;
+    let o = f.construct(&mut cx, [zero.upcast()])?;
     let get_utc_full_year_method = o
         .get(&mut cx, "getUTCFullYear")?
         .downcast::<JsFunction, _>(&mut cx)

diff --git a/test/napi/src/js/threads.rs b/test/napi/src/js/threads.rs
@@ -45,7 +45,7 @@ pub fn multi_threaded_callback(mut cx: FunctionContext) -> JsResult<JsUndefined>
             channel.send(move |mut cx| {
                 let callback = callback.into_inner(&mut cx);
                 let this = cx.undefined();
-                let args = vec![cx.number(i as f64)];
+                let args = [cx.number(i as f64).upcast()];
 
                 callback.call(&mut cx, this, args)?;
 
@@ -78,7 +78,7 @@ impl AsyncGreeter {
             channel.send(|mut cx| {
                 let callback = callback.into_inner(&mut cx);
                 let this = cx.undefined();
-                let args = vec![cx.string(greeting)];
+                let args = [cx.string(greeting).upcast()];
 
                 callback.call(&mut cx, this, args)?;
 
@@ -163,7 +163,7 @@ pub fn drop_global_queue(mut cx: FunctionContext) -> JsResult<JsUndefined> {
                 self.channel.send(|mut cx| {
                     let callback = callback.into_inner(&mut cx);
                     let this = cx.undefined();
-                    let args = vec![cx.undefined()];
+                    let args = [cx.undefined().upcast()];
 
                     callback.call(&mut cx, this, args)?;
 
@@ -207,7 +207,7 @@ pub fn channel_join(mut cx: FunctionContext) -> JsResult<JsUndefined> {
 
                 get_message
                     .into_inner(&mut cx)
-                    .call::<_, _, JsValue, _>(&mut cx, this, [])?
+                    .call(&mut cx, this, [])?
                     .downcast_or_throw::<JsString, _>(&mut cx)
                     .map(|v| v.value(&mut cx))
             })
@@ -220,7 +220,7 @@ pub fn channel_join(mut cx: FunctionContext) -> JsResult<JsUndefined> {
         // Call back to JavaScript with the response
         channel.send(move |mut cx| {
             let this = cx.undefined();
-            let args = [cx.string(response)];
+            let args = [cx.string(response).upcast()];
 
             callback.into_inner(&mut cx).call(&mut cx, this, args)?;