Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refined .call() and .construct() API #817

Closed
wants to merge 8 commits into from
4 changes: 2 additions & 2 deletions crates/neon-runtime/src/napi/fun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 _);

Expand Down
4 changes: 2 additions & 2 deletions crates/neon-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,8 @@
//!
//! while !done {
//! done = cx.execute_scoped(|mut cx| { // temporary scope
//! let args: Vec<Handle<JsValue>> = vec![];
//! let obj = next.call(&mut cx, iterator, args)? // temporary object
//! .downcast_or_throw::<JsObject, _>(&mut cx)?;
//! let obj: Handle<JsObject> = next // temporary object
//! .call(&mut cx, iterator, ())?;
//! let number = obj.get(&mut cx, "value")? // temporary number
//! .downcast_or_throw::<JsNumber, _>(&mut cx)?
//! .value(&mut cx);
Expand Down
2 changes: 1 addition & 1 deletion src/event/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl EventHandler {
{
self.schedule_with(move |cx, this, callback| {
let args = arg_cb(cx);
let _result = callback.call(cx, this, args);
let _result = callback.exec(cx, this, args);
})
}

Expand Down
8 changes: 4 additions & 4 deletions src/object/class/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use crate::context::{Context, Lock};
use crate::handle::{Handle, Managed};
use crate::object::{Object, This};
use crate::result::{JsResult, NeonResult, Throw};
use crate::types::internal::{Callback, ValueInternal};
use crate::types::function::Arguments;
use crate::types::private::{Callback, ValueInternal};
use crate::types::{build, JsFunction, JsValue, Value};
use neon_runtime;
use neon_runtime::raw;
Expand Down Expand Up @@ -103,10 +104,9 @@ pub trait Class: Managed + Any {
}

/// Convenience method for constructing new instances of this class without having to extract the constructor function.
fn new<'a, 'b, C: Context<'a>, A, AS>(cx: &mut C, args: AS) -> JsResult<'a, Self>
fn new<'a, 'b, C: Context<'a>, A>(cx: &mut C, args: A) -> JsResult<'a, Self>
where
A: Value + 'b,
AS: IntoIterator<Item = Handle<'b, A>>,
A: Arguments<'b>,
{
let constructor = Self::constructor(cx)?;
constructor.construct(cx, args)
Expand Down
2 changes: 1 addition & 1 deletion src/types/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::context::internal::Env;
use crate::context::{Context, Lock};
use crate::handle::Managed;
use crate::result::JsResult;
use crate::types::internal::ValueInternal;
use crate::types::private::ValueInternal;
use crate::types::{build, Object, Value};
use neon_runtime;
use neon_runtime::raw;
Expand Down
4 changes: 2 additions & 2 deletions src/types/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::context::internal::Env;
use crate::context::{Context, FinalizeContext};
use crate::handle::{Handle, Managed};
use crate::object::Object;
use crate::types::internal::ValueInternal;
use crate::types::private::ValueInternal;
use crate::types::Value;

type BoxAny = Box<dyn Any + Send + 'static>;
Expand Down Expand Up @@ -294,7 +294,7 @@ impl<'a, T: Send + 'static> Deref for JsBox<T> {
/// cx.number(self.1).upcast(),
/// ];
///
/// emit.call(cx, global, args).unwrap();
/// emit.exec(cx, global, args).unwrap();
/// }
/// }
/// ```
Expand Down
2 changes: 1 addition & 1 deletion src/types/buffer/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use neon_runtime::{raw, TypedArrayType};
use crate::context::{internal::Env, Context};
use crate::handle::{Handle, Managed};
use crate::result::{JsResult, Throw};
use crate::types::{internal::ValueInternal, Object, Value};
use crate::types::{private::ValueInternal, Object, Value};

use super::lock::{Ledger, Lock};
use super::{private, BorrowError, Ref, RefMut, TypedArray};
Expand Down
2 changes: 1 addition & 1 deletion src/types/date.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Value, ValueInternal};
use super::{private::ValueInternal, Value};
use crate::context::internal::Env;
use crate::context::Context;
use crate::handle::{Handle, Managed};
Expand Down
2 changes: 1 addition & 1 deletion src/types/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use neon_runtime::raw;
use crate::context::internal::Env;
use crate::context::Context;
use crate::result::{NeonResult, Throw};
use crate::types::internal::ValueInternal;
use crate::types::private::ValueInternal;
use crate::types::utf8::Utf8;
use crate::types::{build, Handle, Managed, Object, Value};

Expand Down
140 changes: 140 additions & 0 deletions src/types/function/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
//! Types and traits for working with JavaScript functions.

use crate::handle::Handle;
use crate::types::Value;

use smallvec::smallvec;

pub(crate) mod private;

/// The trait for specifying arguments for a function call. This trait is sealed and cannot
/// be implemented by types outside of the Neon crate.
///
/// **Note:** This trait is implemented for tuples of up to 32 JavaScript values,
/// but for the sake of brevity, only tuples up to size 8 are shown in this documentation.
pub trait Arguments<'a>: private::ArgumentsInternal<'a> {}

impl<'a, T: Value> private::ArgumentsInternal<'a> for Vec<Handle<'a, T>> {
fn into_args_vec(self) -> private::ArgsVec<'a> {
let mut args = smallvec![];
for arg in self {
args.push(arg.upcast());
}
args
Comment on lines +19 to +23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust is sometimes able to optimize iterators a little better. SmallVec implements from_iter, so it can be used with collect.

Suggested change
let mut args = smallvec![];
for arg in self {
args.push(arg.upcast());
}
args
self.into_iter().map(|v| v.upcast()).collect()

}
}

impl<'a, T: Value> Arguments<'a> for Vec<Handle<'a, T>> {}

impl<'a, T: Value, const N: usize> private::ArgumentsInternal<'a> for [Handle<'a, T>; N] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const generics will require a fairly recent version of Rust (1.51, released in March). Given that we're copying anyway, maybe this only needs to be implemented over slices?

I think this and Vec could be deleted and only leave an implementation for &[Handle<'a, T>].

fn into_args_vec(self) -> private::ArgsVec<'a> {
let mut args = smallvec![];
for arg in self {
args.push(arg.upcast());
}
args
Comment on lines +31 to +35
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut args = smallvec![];
for arg in self {
args.push(arg.upcast());
}
args
self.iter().map(|v| v.upcast()).collect()

}
}

impl<'a, T: Value, const N: usize> Arguments<'a> for [Handle<'a, T>; N] {}

impl<'a> private::ArgumentsInternal<'a> for () {
fn into_args_vec(self) -> private::ArgsVec<'a> {
smallvec![]
}
}

impl<'a> Arguments<'a> for () {}

macro_rules! impl_arguments {
{
[ $(($tprefix:ident, $vprefix:ident), )* ];
[];
} => {};

{
[ $(($tprefix:ident, $vprefix:ident), )* ];
[ $(#[$attr1:meta])? ($tname1:ident, $vname1:ident), $($(#[$attrs:meta])? ($tnames:ident, $vnames:ident), )* ];
} => {
$(#[$attr1])?
impl<'a, $($tprefix: Value, )* $tname1: Value> private::ArgumentsInternal<'a> for ($(Handle<'a, $tprefix>, )* Handle<'a, $tname1>, ) {
fn into_args_vec(self) -> private::ArgsVec<'a> {
let mut args = smallvec![];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would result in better performance if the items were expanded in the smallvec macro instead of pushing one by one.

let ($($vprefix, )* $vname1, ) = self;
$(args.push($vprefix.upcast());)*
args.push($vname1.upcast());
args
}
}

$(#[$attr1])?
impl<'a, $($tprefix: Value, )* $tname1: Value> Arguments<'a> for ($(Handle<'a, $tprefix>, )* Handle<'a, $tname1>, ) {}

impl_arguments! {
[ $(($tprefix, $vprefix), )* ($tname1, $vname1), ];
[ $($(#[$attrs])? ($tnames, $vnames), )* ];
}
};
}

impl_arguments! {
[];
[
(V1, v1),
(V2, v2),
(V3, v3),
(V4, v4),
(V5, v5),
(V6, v6),
(V7, v7),
(V8, v8),
#[doc(hidden)]
(V9, v9),
#[doc(hidden)]
(V10, v10),
#[doc(hidden)]
(V11, v11),
#[doc(hidden)]
(V12, v12),
#[doc(hidden)]
(V13, v13),
#[doc(hidden)]
(V14, v14),
#[doc(hidden)]
(V15, v15),
#[doc(hidden)]
(V16, v16),
#[doc(hidden)]
(V17, v17),
#[doc(hidden)]
(V18, v18),
#[doc(hidden)]
(V19, v19),
#[doc(hidden)]
(V20, v20),
#[doc(hidden)]
(V21, v21),
#[doc(hidden)]
(V22, v22),
#[doc(hidden)]
(V23, v23),
#[doc(hidden)]
(V24, v24),
#[doc(hidden)]
(V25, v25),
#[doc(hidden)]
(V26, v26),
#[doc(hidden)]
(V27, v27),
#[doc(hidden)]
(V28, v28),
#[doc(hidden)]
(V29, v29),
#[doc(hidden)]
(V30, v30),
#[doc(hidden)]
(V31, v31),
#[doc(hidden)]
(V32, v32),
];
}
11 changes: 11 additions & 0 deletions src/types/function/private.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use crate::handle::Handle;
use crate::types::JsValue;

use smallvec::SmallVec;

pub type ArgsVec<'a> = SmallVec<[Handle<'a, JsValue>; 8]>;

/// This type marks the `Arguments` trait as sealed.
pub trait ArgumentsInternal<'a> {
fn into_args_vec(self) -> ArgsVec<'a>;
}
Loading