From b06c56a4d6d200cbe46d3cae22de46f02700e3b2 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Thu, 30 May 2024 13:32:34 -0400 Subject: [PATCH 1/7] Fix clippy lint --- crates/neon/src/event/channel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/neon/src/event/channel.rs b/crates/neon/src/event/channel.rs index a665e0a82..25359977b 100644 --- a/crates/neon/src/event/channel.rs +++ b/crates/neon/src/event/channel.rs @@ -161,7 +161,7 @@ impl Channel { let callback = Box::new(move |env| { let env = unsafe { mem::transmute(env) }; - // Note: It is sufficient to use `TaskContext`'s `InheritedHandleScope` because + // Note: It is sufficient to use `TaskContext` because // N-API creates a `HandleScope` before calling the callback. TaskContext::with_context(env, move |cx| { // Error can be ignored; it only means the user didn't join From d28247339c7be3900df48ad0dc82f054b1509d7b Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Thu, 30 May 2024 14:03:31 -0400 Subject: [PATCH 2/7] Replace all generic contexts with a single Ctx type --- crates/neon/src/context/internal.rs | 8 +- crates/neon/src/context/mod.rs | 286 +++++++++++++--------------- crates/neon/src/types_impl/boxed.rs | 2 +- 3 files changed, 137 insertions(+), 159 deletions(-) diff --git a/crates/neon/src/context/internal.rs b/crates/neon/src/context/internal.rs index 1f8f55cfc..8c242ee34 100644 --- a/crates/neon/src/context/internal.rs +++ b/crates/neon/src/context/internal.rs @@ -1,7 +1,7 @@ use std::{cell::RefCell, ffi::c_void, mem::MaybeUninit}; use crate::{ - context::ModuleContext, + context::{Ctx, ModuleContext}, handle::Handle, result::NeonResult, sys::{self, raw}, @@ -46,8 +46,10 @@ impl Env { } } -pub trait ContextInternal<'a>: Sized { - fn env(&self) -> Env; +pub trait ContextInternal<'cx>: AsRef> + AsMut> + Sized { + fn env(&self) -> Env { + self.as_ref().env() + } } fn default_main(mut cx: ModuleContext) -> NeonResult<()> { diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs index 0d936ed3f..01102f63c 100644 --- a/crates/neon/src/context/mod.rs +++ b/crates/neon/src/context/mod.rs @@ -175,10 +175,85 @@ use crate::types::date::{DateError, JsDate}; #[cfg(feature = "napi-6")] use crate::lifecycle::InstanceData; +/// An execution context of a task completion callback. +pub type TaskContext<'cx> = Ctx<'cx>; + +/// An execution context of a scope created by [`Context::execute_scoped()`](Context::execute_scoped). +pub type ExecuteContext<'cx> = Ctx<'cx>; + +/// An execution context of a scope created by [`Context::compute_scoped()`](Context::compute_scoped). +pub type ComputeContext<'cx> = Ctx<'cx>; + +/// A view of the JS engine in the context of a finalize method on garbage collection +pub type FinalizeContext<'cx> = Ctx<'cx>; + +/// An execution context constructed from a raw [`Env`](crate::sys::bindings::Env). +#[cfg(feature = "sys")] +#[cfg_attr(docsrs, doc(cfg(feature = "sys")))] +pub type SysContext<'cx> = Ctx<'cx>; + +/// Context representing access to the JavaScript runtime +pub struct Ctx<'cx> { + env: Env, + _phantom_inner: PhantomData<&'cx ()>, +} + +impl<'cx> Ctx<'cx> { + /// Creates a context from a raw `Env`. + /// + /// # Safety + /// + /// Once a `SysContext` has been created, it is unsafe to use + /// the `Env`. The handle scope for the `Env` must be valid for + /// the lifetime `'cx`. + #[cfg(feature = "sys")] + #[cfg_attr(docsrs, doc(cfg(feature = "sys")))] + pub unsafe fn from_raw(env: sys::Env) -> Self { + Self { + env: env.into(), + _phantom_inner: PhantomData, + } + } + + fn new(env: Env) -> Self { + Self { + env, + _phantom_inner: PhantomData, + } + } + + pub(crate) fn with_context FnOnce(Ctx<'b>) -> T>(env: Env, f: F) -> T { + f(Self { + env, + _phantom_inner: PhantomData, + }) + } + + fn env(&self) -> Env { + self.env + } +} + +impl<'cx> AsRef> for Ctx<'cx> { + fn as_ref(&self) -> &Ctx<'cx> { + self + } +} + +impl<'cx> AsMut> for Ctx<'cx> { + fn as_mut(&mut self) -> &mut Ctx<'cx> { + self + } +} + +impl<'cx> ContextInternal<'cx> for Ctx<'cx> {} + +impl<'cx> Context<'cx> for Ctx<'cx> {} + #[repr(C)] -pub(crate) struct CallbackInfo<'a> { +pub(crate) struct CallbackInfo<'cx> { info: raw::FunctionCallbackInfo, - _lifetime: PhantomData<&'a raw::FunctionCallbackInfo>, + _lifetime: PhantomData<&'cx raw::FunctionCallbackInfo>, } impl CallbackInfo<'_> { @@ -280,10 +355,7 @@ pub trait Context<'a>: ContextInternal<'a> { { let env = self.env(); let scope = unsafe { HandleScope::new(env.to_raw()) }; - let result = f(ExecuteContext { - env, - _phantom_inner: PhantomData, - }); + let result = f(Ctx::new(env)); drop(scope); @@ -303,10 +375,7 @@ pub trait Context<'a>: ContextInternal<'a> { { let env = self.env(); let scope = unsafe { EscapableHandleScope::new(env.to_raw()) }; - let cx = ComputeContext { - env, - phantom_inner: PhantomData, - }; + let cx = Ctx::new(env); let escapee = unsafe { scope.escape(f(cx)?.to_local()) }; @@ -551,20 +620,35 @@ pub trait Context<'a>: ContextInternal<'a> { } /// An execution context of module initialization. -pub struct ModuleContext<'a> { - env: Env, - exports: Handle<'a, JsObject>, +pub struct ModuleContext<'cx> { + cx: Ctx<'cx>, + exports: Handle<'cx, JsObject>, +} + +impl<'cx> AsRef> for ModuleContext<'cx> { + fn as_ref(&self) -> &Ctx<'cx> { + &self.cx + } +} + +impl<'cx> AsMut> for ModuleContext<'cx> { + fn as_mut(&mut self) -> &mut Ctx<'cx> { + &mut self.cx + } } -impl<'a> UnwindSafe for ModuleContext<'a> {} +impl<'cx> UnwindSafe for ModuleContext<'cx> {} -impl<'a> ModuleContext<'a> { +impl<'cx> ModuleContext<'cx> { pub(crate) fn with FnOnce(ModuleContext<'b>) -> T>( env: Env, - exports: Handle<'a, JsObject>, + exports: Handle<'cx, JsObject>, f: F, ) -> T { - f(ModuleContext { env, exports }) + f(ModuleContext { + cx: Ctx::new(env), + exports, + }) } #[cfg(not(feature = "napi-5"))] @@ -600,60 +684,40 @@ impl<'a> ModuleContext<'a> { } /// Produces a handle to a module's exports object. - pub fn exports_object(&mut self) -> JsResult<'a, JsObject> { + pub fn exports_object(&mut self) -> JsResult<'cx, JsObject> { Ok(self.exports) } } -impl<'a> ContextInternal<'a> for ModuleContext<'a> { - fn env(&self) -> Env { - self.env - } -} +impl<'cx> ContextInternal<'cx> for ModuleContext<'cx> {} -impl<'a> Context<'a> for ModuleContext<'a> {} +impl<'cx> Context<'cx> for ModuleContext<'cx> {} -/// An execution context of a scope created by [`Context::execute_scoped()`](Context::execute_scoped). -pub struct ExecuteContext<'a> { - env: Env, - _phantom_inner: PhantomData<&'a ()>, -} - -impl<'a> ContextInternal<'a> for ExecuteContext<'a> { - fn env(&self) -> Env { - self.env - } -} - -impl<'a> Context<'a> for ExecuteContext<'a> {} +/// An execution context of a function call. +/// +/// The type parameter `T` is the type of the `this`-binding. +pub struct FunctionContext<'cx> { + cx: Ctx<'cx>, + info: &'cx CallbackInfo<'cx>, -/// An execution context of a scope created by [`Context::compute_scoped()`](Context::compute_scoped). -pub struct ComputeContext<'a> { - env: Env, - phantom_inner: PhantomData<&'a ()>, + arguments: Option, } -impl<'a> ContextInternal<'a> for ComputeContext<'a> { - fn env(&self) -> Env { - self.env +impl<'cx> AsRef> for FunctionContext<'cx> { + fn as_ref(&self) -> &Ctx<'cx> { + &self.cx } } -impl<'a> Context<'a> for ComputeContext<'a> {} - -/// An execution context of a function call. -/// -/// The type parameter `T` is the type of the `this`-binding. -pub struct FunctionContext<'a> { - env: Env, - info: &'a CallbackInfo<'a>, - - arguments: Option, +impl<'cx> AsMut> for FunctionContext<'cx> { + fn as_mut(&mut self) -> &mut Ctx<'cx> { + &mut self.cx + } } -impl<'a> UnwindSafe for FunctionContext<'a> {} +impl<'cx> UnwindSafe for FunctionContext<'cx> {} -impl<'a> FunctionContext<'a> { +impl<'cx> FunctionContext<'cx> { /// Indicates whether the function was called with `new`. pub fn kind(&self) -> CallKind { self.info.kind(self) @@ -661,11 +725,11 @@ impl<'a> FunctionContext<'a> { pub(crate) fn with FnOnce(FunctionContext<'b>) -> U>( env: Env, - info: &'a CallbackInfo<'a>, + info: &'cx CallbackInfo<'cx>, f: F, ) -> U { f(FunctionContext { - env, + cx: Ctx::new(env), info, arguments: None, }) @@ -682,7 +746,7 @@ impl<'a> FunctionContext<'a> { } /// Produces the `i`th argument, or `None` if `i` is greater than or equal to `self.len()`. - pub fn argument_opt(&mut self, i: usize) -> Option> { + pub fn argument_opt(&mut self, i: usize) -> Option> { let argv = if let Some(argv) = self.arguments.as_ref() { argv } else { @@ -695,7 +759,7 @@ impl<'a> FunctionContext<'a> { } /// Produces the `i`th argument and casts it to the type `V`, or throws an exception if `i` is greater than or equal to `self.len()` or cannot be cast to `V`. - pub fn argument(&mut self, i: usize) -> JsResult<'a, V> { + pub fn argument(&mut self, i: usize) -> JsResult<'cx, V> { match self.argument_opt(i) { Some(v) => v.downcast_or_throw(self), None => self.throw_type_error("not enough arguments"), @@ -706,12 +770,12 @@ impl<'a> FunctionContext<'a> { /// Equivalent to calling `cx.this_value().downcast_or_throw(&mut cx)`. /// /// Throws an exception if the value is a different type. - pub fn this(&mut self) -> JsResult<'a, T> { + pub fn this(&mut self) -> JsResult<'cx, T> { self.this_value().downcast_or_throw(self) } /// Produces a handle to the function's [`this`-binding](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this#function_context). - pub fn this_value(&mut self) -> Handle<'a, JsValue> { + pub fn this_value(&mut self) -> Handle<'cx, JsValue> { JsValue::new_internal(self.info.this(self)) } @@ -731,7 +795,7 @@ impl<'a> FunctionContext<'a> { /// ``` pub fn args(&mut self) -> NeonResult where - T: FromArgs<'a>, + T: FromArgs<'cx>, { T::from_args(self) } @@ -755,104 +819,16 @@ impl<'a> FunctionContext<'a> { /// ``` pub fn args_opt(&mut self) -> NeonResult> where - T: FromArgs<'a>, + T: FromArgs<'cx>, { T::from_args_opt(self) } - pub(crate) fn argv(&mut self) -> [Handle<'a, JsValue>; N] { + pub(crate) fn argv(&mut self) -> [Handle<'cx, JsValue>; N] { self.info.argv_exact(self) } } -impl<'a> ContextInternal<'a> for FunctionContext<'a> { - fn env(&self) -> Env { - self.env - } -} - -impl<'a> Context<'a> for FunctionContext<'a> {} +impl<'cx> ContextInternal<'cx> for FunctionContext<'cx> {} -/// An execution context of a task completion callback. -pub struct TaskContext<'a> { - env: Env, - _phantom_inner: PhantomData<&'a ()>, -} - -impl<'a> TaskContext<'a> { - pub(crate) fn with_context FnOnce(TaskContext<'b>) -> T>(env: Env, f: F) -> T { - f(Self { - env, - _phantom_inner: PhantomData, - }) - } -} - -impl<'a> ContextInternal<'a> for TaskContext<'a> { - fn env(&self) -> Env { - self.env - } -} - -impl<'a> Context<'a> for TaskContext<'a> {} - -/// A view of the JS engine in the context of a finalize method on garbage collection -pub(crate) struct FinalizeContext<'a> { - env: Env, - _phantom_inner: PhantomData<&'a ()>, -} - -impl<'a> FinalizeContext<'a> { - pub(crate) fn with FnOnce(FinalizeContext<'b>) -> T>(env: Env, f: F) -> T { - f(Self { - env, - _phantom_inner: PhantomData, - }) - } -} - -impl<'a> ContextInternal<'a> for FinalizeContext<'a> { - fn env(&self) -> Env { - self.env - } -} - -impl<'a> Context<'a> for FinalizeContext<'a> {} - -#[cfg(feature = "sys")] -#[cfg_attr(docsrs, doc(cfg(feature = "sys")))] -/// An execution context constructed from a raw [`Env`](crate::sys::bindings::Env). -pub struct SysContext<'cx> { - env: Env, - _phantom_inner: PhantomData<&'cx ()>, -} - -#[cfg(feature = "sys")] -impl<'cx> SysContext<'cx> { - /// Creates a context from a raw `Env`. - /// - /// # Safety - /// - /// Once a `SysContext` has been created, it is unsafe to use - /// the `Env`. The handle scope for the `Env` must be valid for - /// the lifetime `'cx`. - pub unsafe fn from_raw(env: sys::Env) -> Self { - Self { - env: env.into(), - _phantom_inner: PhantomData, - } - } -} - -#[cfg(feature = "sys")] -impl<'cx> SysContext<'cx> {} - -#[cfg(feature = "sys")] -impl<'cx> ContextInternal<'cx> for SysContext<'cx> { - fn env(&self) -> Env { - self.env - } -} - -#[cfg(feature = "sys")] -impl<'cx> Context<'cx> for SysContext<'cx> {} +impl<'cx> Context<'cx> for FunctionContext<'cx> {} diff --git a/crates/neon/src/types_impl/boxed.rs b/crates/neon/src/types_impl/boxed.rs index e54330757..adabf4271 100644 --- a/crates/neon/src/types_impl/boxed.rs +++ b/crates/neon/src/types_impl/boxed.rs @@ -244,7 +244,7 @@ impl JsBox { let data = *data.downcast::().unwrap(); let env = unsafe { std::mem::transmute(env) }; - FinalizeContext::with(env, move |mut cx| data.finalize(&mut cx)); + FinalizeContext::with_context(env, move |mut cx| data.finalize(&mut cx)); } let v = Box::new(value) as BoxAny; From af6c759039ba322f2141b0377b573317c9e49b53 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Mon, 3 Jun 2024 15:05:57 -0400 Subject: [PATCH 3/7] Rename Ctx to Cx and add Deref/DerefMut for coercions --- crates/neon/src/context/internal.rs | 4 +- crates/neon/src/context/mod.rs | 91 ++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/crates/neon/src/context/internal.rs b/crates/neon/src/context/internal.rs index 8c242ee34..f1b8f49bb 100644 --- a/crates/neon/src/context/internal.rs +++ b/crates/neon/src/context/internal.rs @@ -1,7 +1,7 @@ use std::{cell::RefCell, ffi::c_void, mem::MaybeUninit}; use crate::{ - context::{Ctx, ModuleContext}, + context::{Cx, ModuleContext}, handle::Handle, result::NeonResult, sys::{self, raw}, @@ -46,7 +46,7 @@ impl Env { } } -pub trait ContextInternal<'cx>: AsRef> + AsMut> + Sized { +pub trait ContextInternal<'cx>: AsRef> + AsMut> + Sized { fn env(&self) -> Env { self.as_ref().env() } diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs index 01102f63c..9490757aa 100644 --- a/crates/neon/src/context/mod.rs +++ b/crates/neon/src/context/mod.rs @@ -141,7 +141,12 @@ pub(crate) mod internal; -use std::{convert::Into, marker::PhantomData, panic::UnwindSafe}; +use std::{ + convert::Into, + marker::PhantomData, + ops::{Deref, DerefMut}, + panic::UnwindSafe, +}; pub use crate::types::buffer::lock::Lock; @@ -176,29 +181,29 @@ use crate::types::date::{DateError, JsDate}; use crate::lifecycle::InstanceData; /// An execution context of a task completion callback. -pub type TaskContext<'cx> = Ctx<'cx>; +pub type TaskContext<'cx> = Cx<'cx>; /// An execution context of a scope created by [`Context::execute_scoped()`](Context::execute_scoped). -pub type ExecuteContext<'cx> = Ctx<'cx>; +pub type ExecuteContext<'cx> = Cx<'cx>; /// An execution context of a scope created by [`Context::compute_scoped()`](Context::compute_scoped). -pub type ComputeContext<'cx> = Ctx<'cx>; +pub type ComputeContext<'cx> = Cx<'cx>; /// A view of the JS engine in the context of a finalize method on garbage collection -pub type FinalizeContext<'cx> = Ctx<'cx>; +pub type FinalizeContext<'cx> = Cx<'cx>; /// An execution context constructed from a raw [`Env`](crate::sys::bindings::Env). #[cfg(feature = "sys")] #[cfg_attr(docsrs, doc(cfg(feature = "sys")))] -pub type SysContext<'cx> = Ctx<'cx>; +pub type SysContext<'cx> = Cx<'cx>; /// Context representing access to the JavaScript runtime -pub struct Ctx<'cx> { +pub struct Cx<'cx> { env: Env, _phantom_inner: PhantomData<&'cx ()>, } -impl<'cx> Ctx<'cx> { +impl<'cx> Cx<'cx> { /// Creates a context from a raw `Env`. /// /// # Safety @@ -222,7 +227,7 @@ impl<'cx> Ctx<'cx> { } } - pub(crate) fn with_context FnOnce(Ctx<'b>) -> T>(env: Env, f: F) -> T { + pub(crate) fn with_context FnOnce(Cx<'b>) -> T>(env: Env, f: F) -> T { f(Self { env, _phantom_inner: PhantomData, @@ -234,21 +239,21 @@ impl<'cx> Ctx<'cx> { } } -impl<'cx> AsRef> for Ctx<'cx> { - fn as_ref(&self) -> &Ctx<'cx> { +impl<'cx> AsRef> for Cx<'cx> { + fn as_ref(&self) -> &Cx<'cx> { self } } -impl<'cx> AsMut> for Ctx<'cx> { - fn as_mut(&mut self) -> &mut Ctx<'cx> { +impl<'cx> AsMut> for Cx<'cx> { + fn as_mut(&mut self) -> &mut Cx<'cx> { self } } -impl<'cx> ContextInternal<'cx> for Ctx<'cx> {} +impl<'cx> ContextInternal<'cx> for Cx<'cx> {} -impl<'cx> Context<'cx> for Ctx<'cx> {} +impl<'cx> Context<'cx> for Cx<'cx> {} #[repr(C)] pub(crate) struct CallbackInfo<'cx> { @@ -355,7 +360,7 @@ pub trait Context<'a>: ContextInternal<'a> { { let env = self.env(); let scope = unsafe { HandleScope::new(env.to_raw()) }; - let result = f(Ctx::new(env)); + let result = f(Cx::new(env)); drop(scope); @@ -375,7 +380,7 @@ pub trait Context<'a>: ContextInternal<'a> { { let env = self.env(); let scope = unsafe { EscapableHandleScope::new(env.to_raw()) }; - let cx = Ctx::new(env); + let cx = Cx::new(env); let escapee = unsafe { scope.escape(f(cx)?.to_local()) }; @@ -621,22 +626,36 @@ pub trait Context<'a>: ContextInternal<'a> { /// An execution context of module initialization. pub struct ModuleContext<'cx> { - cx: Ctx<'cx>, + cx: Cx<'cx>, exports: Handle<'cx, JsObject>, } -impl<'cx> AsRef> for ModuleContext<'cx> { - fn as_ref(&self) -> &Ctx<'cx> { +impl<'cx> Deref for ModuleContext<'cx> { + type Target = Cx<'cx>; + + fn deref(&self) -> &Self::Target { &self.cx } } -impl<'cx> AsMut> for ModuleContext<'cx> { - fn as_mut(&mut self) -> &mut Ctx<'cx> { +impl<'cx> DerefMut for ModuleContext<'cx> { + fn deref_mut(&mut self) -> &mut Self::Target { &mut self.cx } } +impl<'cx> AsRef> for ModuleContext<'cx> { + fn as_ref(&self) -> &Cx<'cx> { + self + } +} + +impl<'cx> AsMut> for ModuleContext<'cx> { + fn as_mut(&mut self) -> &mut Cx<'cx> { + self + } +} + impl<'cx> UnwindSafe for ModuleContext<'cx> {} impl<'cx> ModuleContext<'cx> { @@ -646,7 +665,7 @@ impl<'cx> ModuleContext<'cx> { f: F, ) -> T { f(ModuleContext { - cx: Ctx::new(env), + cx: Cx::new(env), exports, }) } @@ -697,24 +716,38 @@ impl<'cx> Context<'cx> for ModuleContext<'cx> {} /// /// The type parameter `T` is the type of the `this`-binding. pub struct FunctionContext<'cx> { - cx: Ctx<'cx>, + cx: Cx<'cx>, info: &'cx CallbackInfo<'cx>, arguments: Option, } -impl<'cx> AsRef> for FunctionContext<'cx> { - fn as_ref(&self) -> &Ctx<'cx> { +impl<'cx> Deref for FunctionContext<'cx> { + type Target = Cx<'cx>; + + fn deref(&self) -> &Self::Target { &self.cx } } -impl<'cx> AsMut> for FunctionContext<'cx> { - fn as_mut(&mut self) -> &mut Ctx<'cx> { +impl<'cx> DerefMut for FunctionContext<'cx> { + fn deref_mut(&mut self) -> &mut Self::Target { &mut self.cx } } +impl<'cx> AsRef> for FunctionContext<'cx> { + fn as_ref(&self) -> &Cx<'cx> { + self + } +} + +impl<'cx> AsMut> for FunctionContext<'cx> { + fn as_mut(&mut self) -> &mut Cx<'cx> { + self + } +} + impl<'cx> UnwindSafe for FunctionContext<'cx> {} impl<'cx> FunctionContext<'cx> { @@ -729,7 +762,7 @@ impl<'cx> FunctionContext<'cx> { f: F, ) -> U { f(FunctionContext { - cx: Ctx::new(env), + cx: Cx::new(env), info, arguments: None, }) From c000f75931a2c9c4e1084411b4cfdee4402fe522 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Mon, 3 Jun 2024 15:11:04 -0400 Subject: [PATCH 4/7] Remove unnecessary AsRef and AsMut --- crates/neon/src/context/internal.rs | 8 ++--- crates/neon/src/context/mod.rs | 52 +++++++---------------------- 2 files changed, 15 insertions(+), 45 deletions(-) diff --git a/crates/neon/src/context/internal.rs b/crates/neon/src/context/internal.rs index f1b8f49bb..bf872d2e5 100644 --- a/crates/neon/src/context/internal.rs +++ b/crates/neon/src/context/internal.rs @@ -1,7 +1,7 @@ use std::{cell::RefCell, ffi::c_void, mem::MaybeUninit}; use crate::{ - context::{Cx, ModuleContext}, + context::ModuleContext, handle::Handle, result::NeonResult, sys::{self, raw}, @@ -46,10 +46,8 @@ impl Env { } } -pub trait ContextInternal<'cx>: AsRef> + AsMut> + Sized { - fn env(&self) -> Env { - self.as_ref().env() - } +pub trait ContextInternal<'cx>: Sized { + fn env(&self) -> Env; } fn default_main(mut cx: ModuleContext) -> NeonResult<()> { diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs index 9490757aa..6c90e7cd6 100644 --- a/crates/neon/src/context/mod.rs +++ b/crates/neon/src/context/mod.rs @@ -233,26 +233,14 @@ impl<'cx> Cx<'cx> { _phantom_inner: PhantomData, }) } +} +impl<'cx> ContextInternal<'cx> for Cx<'cx> { fn env(&self) -> Env { self.env } } -impl<'cx> AsRef> for Cx<'cx> { - fn as_ref(&self) -> &Cx<'cx> { - self - } -} - -impl<'cx> AsMut> for Cx<'cx> { - fn as_mut(&mut self) -> &mut Cx<'cx> { - self - } -} - -impl<'cx> ContextInternal<'cx> for Cx<'cx> {} - impl<'cx> Context<'cx> for Cx<'cx> {} #[repr(C)] @@ -644,18 +632,6 @@ impl<'cx> DerefMut for ModuleContext<'cx> { } } -impl<'cx> AsRef> for ModuleContext<'cx> { - fn as_ref(&self) -> &Cx<'cx> { - self - } -} - -impl<'cx> AsMut> for ModuleContext<'cx> { - fn as_mut(&mut self) -> &mut Cx<'cx> { - self - } -} - impl<'cx> UnwindSafe for ModuleContext<'cx> {} impl<'cx> ModuleContext<'cx> { @@ -708,7 +684,11 @@ impl<'cx> ModuleContext<'cx> { } } -impl<'cx> ContextInternal<'cx> for ModuleContext<'cx> {} +impl<'cx> ContextInternal<'cx> for ModuleContext<'cx> { + fn env(&self) -> Env { + self.cx.env + } +} impl<'cx> Context<'cx> for ModuleContext<'cx> {} @@ -736,18 +716,6 @@ impl<'cx> DerefMut for FunctionContext<'cx> { } } -impl<'cx> AsRef> for FunctionContext<'cx> { - fn as_ref(&self) -> &Cx<'cx> { - self - } -} - -impl<'cx> AsMut> for FunctionContext<'cx> { - fn as_mut(&mut self) -> &mut Cx<'cx> { - self - } -} - impl<'cx> UnwindSafe for FunctionContext<'cx> {} impl<'cx> FunctionContext<'cx> { @@ -862,6 +830,10 @@ impl<'cx> FunctionContext<'cx> { } } -impl<'cx> ContextInternal<'cx> for FunctionContext<'cx> {} +impl<'cx> ContextInternal<'cx> for FunctionContext<'cx> { + fn env(&self) -> Env { + self.cx.env + } +} impl<'cx> Context<'cx> for FunctionContext<'cx> {} From 19143c696e0dec80612a0be8f04de4c821bdd9a8 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Wed, 24 Jul 2024 09:13:23 -0400 Subject: [PATCH 5/7] Mark context aliases as `#[doc(hidden)]` and remove all internal references --- crates/neon/src/context/mod.rs | 11 ++++++++--- crates/neon/src/event/channel.rs | 10 +++++----- crates/neon/src/event/task.rs | 18 +++++++++--------- crates/neon/src/prelude.rs | 8 ++++---- crates/neon/src/sys/mod.rs | 4 ++-- crates/neon/src/types_impl/boxed.rs | 4 ++-- crates/neon/src/types_impl/promise.rs | 14 ++++++-------- 7 files changed, 36 insertions(+), 33 deletions(-) diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs index 6c90e7cd6..717024d92 100644 --- a/crates/neon/src/context/mod.rs +++ b/crates/neon/src/context/mod.rs @@ -180,21 +180,26 @@ use crate::types::date::{DateError, JsDate}; #[cfg(feature = "napi-6")] use crate::lifecycle::InstanceData; +#[doc(hidden)] /// An execution context of a task completion callback. pub type TaskContext<'cx> = Cx<'cx>; +#[doc(hidden)] /// An execution context of a scope created by [`Context::execute_scoped()`](Context::execute_scoped). pub type ExecuteContext<'cx> = Cx<'cx>; +#[doc(hidden)] /// An execution context of a scope created by [`Context::compute_scoped()`](Context::compute_scoped). pub type ComputeContext<'cx> = Cx<'cx>; +#[doc(hidden)] /// A view of the JS engine in the context of a finalize method on garbage collection pub type FinalizeContext<'cx> = Cx<'cx>; /// An execution context constructed from a raw [`Env`](crate::sys::bindings::Env). #[cfg(feature = "sys")] #[cfg_attr(docsrs, doc(cfg(feature = "sys")))] +#[doc(hidden)] pub type SysContext<'cx> = Cx<'cx>; /// Context representing access to the JavaScript runtime @@ -208,7 +213,7 @@ impl<'cx> Cx<'cx> { /// /// # Safety /// - /// Once a `SysContext` has been created, it is unsafe to use + /// Once a [`Cx`] has been created, it is unsafe to use /// the `Env`. The handle scope for the `Env` must be valid for /// the lifetime `'cx`. #[cfg(feature = "sys")] @@ -344,7 +349,7 @@ pub trait Context<'a>: ContextInternal<'a> { fn execute_scoped<'b, T, F>(&mut self, f: F) -> T where 'a: 'b, - F: FnOnce(ExecuteContext<'b>) -> T, + F: FnOnce(Cx<'b>) -> T, { let env = self.env(); let scope = unsafe { HandleScope::new(env.to_raw()) }; @@ -364,7 +369,7 @@ pub trait Context<'a>: ContextInternal<'a> { where 'a: 'b, V: Value, - F: FnOnce(ComputeContext<'b>) -> JsResult<'b, V>, + F: FnOnce(Cx<'b>) -> JsResult<'b, V>, { let env = self.env(); let scope = unsafe { EscapableHandleScope::new(env.to_raw()) }; diff --git a/crates/neon/src/event/channel.rs b/crates/neon/src/event/channel.rs index 25359977b..860292bc6 100644 --- a/crates/neon/src/event/channel.rs +++ b/crates/neon/src/event/channel.rs @@ -7,7 +7,7 @@ use std::{ }; use crate::{ - context::{Context, TaskContext}, + context::{Context, Cx}, result::{NeonResult, ResultExt, Throw}, sys::{raw::Env, tsfn::ThreadsafeFunction}, }; @@ -143,7 +143,7 @@ impl Channel { pub fn send(&self, f: F) -> JoinHandle where T: Send + 'static, - F: FnOnce(TaskContext) -> NeonResult + Send + 'static, + F: FnOnce(Cx) -> NeonResult + Send + 'static, { self.try_send(f).unwrap() } @@ -155,15 +155,15 @@ impl Channel { pub fn try_send(&self, f: F) -> Result, SendError> where T: Send + 'static, - F: FnOnce(TaskContext) -> NeonResult + Send + 'static, + F: FnOnce(Cx) -> NeonResult + Send + 'static, { let (tx, rx) = oneshot::channel(); let callback = Box::new(move |env| { let env = unsafe { mem::transmute(env) }; - // Note: It is sufficient to use `TaskContext` because + // Note: It is sufficient to use `Cx` because // N-API creates a `HandleScope` before calling the callback. - TaskContext::with_context(env, move |cx| { + Cx::with_context(env, move |cx| { // Error can be ignored; it only means the user didn't join let _ = tx.send(f(cx).map_err(Into::into)); }); diff --git a/crates/neon/src/event/task.rs b/crates/neon/src/event/task.rs index 925bacd53..bfbf9b669 100644 --- a/crates/neon/src/event/task.rs +++ b/crates/neon/src/event/task.rs @@ -1,7 +1,7 @@ use std::{panic::resume_unwind, thread}; use crate::{ - context::{internal::Env, Context, TaskContext}, + context::{internal::Env, Context, Cx}, handle::Handle, result::{JsResult, NeonResult}, sys::{async_work, raw}, @@ -44,7 +44,7 @@ where /// of the `execute` callback pub fn and_then(self, complete: F) where - F: FnOnce(TaskContext, O) -> NeonResult<()> + 'static, + F: FnOnce(Cx, O) -> NeonResult<()> + 'static, { let env = self.cx.env(); let execute = self.execute; @@ -61,7 +61,7 @@ where pub fn promise(self, complete: F) -> Handle<'a, JsPromise> where V: Value, - F: FnOnce(TaskContext, O) -> JsResult + 'static, + F: FnOnce(Cx, O) -> JsResult + 'static, { let env = self.cx.env(); let (deferred, promise) = JsPromise::new(self.cx); @@ -78,7 +78,7 @@ fn schedule(env: Env, input: I, data: D) where I: FnOnce() -> O + Send + 'static, O: Send + 'static, - D: FnOnce(TaskContext, O) -> NeonResult<()> + 'static, + D: FnOnce(Cx, O) -> NeonResult<()> + 'static, { unsafe { async_work::schedule(env.to_raw(), input, execute::, complete::, data); @@ -96,7 +96,7 @@ where fn complete(env: raw::Env, output: thread::Result, callback: D) where O: Send + 'static, - D: FnOnce(TaskContext, O) -> NeonResult<()> + 'static, + D: FnOnce(Cx, O) -> NeonResult<()> + 'static, { let output = output.unwrap_or_else(|panic| { // If a panic was caught while executing the task on the Node Worker @@ -104,7 +104,7 @@ where resume_unwind(panic) }); - TaskContext::with_context(env.into(), move |cx| { + Cx::with_context(env.into(), move |cx| { let _ = callback(cx, output); }); } @@ -114,7 +114,7 @@ fn schedule_promise(env: Env, input: I, complete: D, deferred: Defer where I: FnOnce() -> O + Send + 'static, O: Send + 'static, - D: FnOnce(TaskContext, O) -> JsResult + 'static, + D: FnOnce(Cx, O) -> JsResult + 'static, V: Value, { unsafe { @@ -134,12 +134,12 @@ fn complete_promise( (complete, deferred): (D, Deferred), ) where O: Send + 'static, - D: FnOnce(TaskContext, O) -> JsResult + 'static, + D: FnOnce(Cx, O) -> JsResult + 'static, V: Value, { let env = env.into(); - TaskContext::with_context(env, move |cx| { + Cx::with_context(env, move |cx| { deferred.try_catch_settle(cx, move |cx| { let output = output.unwrap_or_else(|panic| resume_unwind(panic)); diff --git a/crates/neon/src/prelude.rs b/crates/neon/src/prelude.rs index 01b87f55e..055ae1f9d 100644 --- a/crates/neon/src/prelude.rs +++ b/crates/neon/src/prelude.rs @@ -2,10 +2,7 @@ #[doc(no_inline)] pub use crate::{ - context::{ - CallKind, ComputeContext, Context, ExecuteContext, FunctionContext, ModuleContext, - TaskContext, - }, + context::{CallKind, Context, Cx, FunctionContext, ModuleContext}, handle::{Handle, Root}, object::Object, result::{JsResult, NeonResult, ResultExt as NeonResultExt}, @@ -18,6 +15,9 @@ pub use crate::{ }, }; +#[doc(hidden)] +pub use crate::context::{ComputeContext, ExecuteContext, TaskContext}; + #[cfg(feature = "napi-4")] #[doc(no_inline)] pub use crate::event::{Channel, SendError}; diff --git a/crates/neon/src/sys/mod.rs b/crates/neon/src/sys/mod.rs index 10bdde4fc..391ce5a91 100644 --- a/crates/neon/src/sys/mod.rs +++ b/crates/neon/src/sys/mod.rs @@ -53,12 +53,12 @@ //! # #[cfg(feature = "sys")] //! # { //! # let env = std::ptr::null_mut().cast(); -//! use neon::{context::SysContext, prelude::*, sys::bindings}; +//! use neon::{context::Cx, prelude::*, sys::bindings}; //! //! let cx = unsafe { //! neon::sys::setup(env); //! -//! SysContext::from_raw(env) +//! Cx::from_raw(env) //! }; //! //! let raw_string: bindings::Value = cx.string("Hello, World!").to_raw(); diff --git a/crates/neon/src/types_impl/boxed.rs b/crates/neon/src/types_impl/boxed.rs index adabf4271..1caf8ae87 100644 --- a/crates/neon/src/types_impl/boxed.rs +++ b/crates/neon/src/types_impl/boxed.rs @@ -4,7 +4,7 @@ use std::{ }; use crate::{ - context::{internal::Env, Context, FinalizeContext}, + context::{internal::Env, Context, Cx}, handle::{internal::TransparentNoCopyWrapper, Handle}, object::Object, sys::{external, raw}, @@ -244,7 +244,7 @@ impl JsBox { let data = *data.downcast::().unwrap(); let env = unsafe { std::mem::transmute(env) }; - FinalizeContext::with_context(env, move |mut cx| data.finalize(&mut cx)); + Cx::with_context(env, move |mut cx| data.finalize(&mut cx)); } let v = Box::new(value) as BoxAny; diff --git a/crates/neon/src/types_impl/promise.rs b/crates/neon/src/types_impl/promise.rs index 913d788ab..0f817854a 100644 --- a/crates/neon/src/types_impl/promise.rs +++ b/crates/neon/src/types_impl/promise.rs @@ -11,7 +11,7 @@ use crate::{ #[cfg(feature = "napi-4")] use crate::{ - context::TaskContext, + context::Cx, event::{Channel, JoinHandle, SendError}, }; @@ -177,9 +177,7 @@ impl JsPromise { where O: Send + 'static, C: Context<'a>, - F: FnOnce(TaskContext, Result, Handle>) -> NeonResult - + Send - + 'static, + F: FnOnce(Cx, Result, Handle>) -> NeonResult + Send + 'static, { let then = self.get::(cx, "then")?; @@ -207,7 +205,7 @@ impl JsPromise { let (f, tx) = take_state(); let v = cx.argument::(0)?; - TaskContext::with_context(cx.env(), move |cx| { + Cx::with_context(cx.env(), move |cx| { // Error indicates that the `Future` has already dropped; ignore let _ = tx.send(f(cx, Ok(v)).map_err(Into::into)); }); @@ -221,7 +219,7 @@ impl JsPromise { let (f, tx) = take_state(); let v = cx.argument::(0)?; - TaskContext::with_context(cx.env(), move |cx| { + Cx::with_context(cx.env(), move |cx| { // Error indicates that the `Future` has already dropped; ignore let _ = tx.send(f(cx, Err(v)).map_err(Into::into)); }); @@ -327,7 +325,7 @@ impl Deferred { ) -> Result, SendError> where V: Value, - F: FnOnce(TaskContext) -> JsResult + Send + 'static, + F: FnOnce(Cx) -> JsResult + Send + 'static, { channel.try_send(move |cx| { self.try_catch_settle(cx, complete); @@ -356,7 +354,7 @@ impl Deferred { pub fn settle_with(self, channel: &Channel, complete: F) -> JoinHandle<()> where V: Value, - F: FnOnce(TaskContext) -> JsResult + Send + 'static, + F: FnOnce(Cx) -> JsResult + Send + 'static, { self.try_settle_with(channel, complete).unwrap() } From bdf01834d0241e919b697d3ba8a143cc816a6931 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Wed, 24 Jul 2024 09:26:20 -0400 Subject: [PATCH 6/7] Add an example of using Cx --- crates/neon/src/context/mod.rs | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs index 717024d92..036d65589 100644 --- a/crates/neon/src/context/mod.rs +++ b/crates/neon/src/context/mod.rs @@ -34,6 +34,31 @@ //! } //! ``` //! +//! ## Writing Generic Helpers +//! +//! Depending on the entrypoint, a user may have a [`FunctionContext`], [`ModuleContext`], or +//! generic [`Cx`]. While it is possible to write a helper that is generic over the [`Context`] +//! trait, it is often simpler to accept a [`Cx`] argument. Due to deref coercion, other contexts +//! may be passed into a function that accepts a reference to [`Cx`]. +//! +//! ``` +//! # use neon::prelude::*; +//! fn log(cx: &mut Cx, msg: &str) -> NeonResult<()> { +//! cx.global::("console")? +//! .call_method_with(cx, "log")? +//! .arg(cx.string(msg)) +//! .exec(cx)?; +//! +//! Ok(()) +//! } +//! +//! fn print(mut cx: FunctionContext) -> JsResult { +//! let msg = cx.argument::(0)?.value(&mut cx); +//! log(&mut cx, &msg)?; +//! Ok(cx.undefined()) +//! } +//! ``` +//! //! ## Memory Management //! //! Because contexts represent the engine at a point in time, they are associated with a @@ -248,6 +273,18 @@ impl<'cx> ContextInternal<'cx> for Cx<'cx> { impl<'cx> Context<'cx> for Cx<'cx> {} +impl<'cx> From> for Cx<'cx> { + fn from(cx: FunctionContext<'cx>) -> Self { + cx.cx + } +} + +impl<'cx> From> for Cx<'cx> { + fn from(cx: ModuleContext<'cx>) -> Self { + cx.cx + } +} + #[repr(C)] pub(crate) struct CallbackInfo<'cx> { info: raw::FunctionCallbackInfo, From d14c1e14363ad242b2235fa782f2bf2b210342f2 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Wed, 24 Jul 2024 11:27:41 -0400 Subject: [PATCH 7/7] Add methods to get Cx to ContextInternal --- crates/neon/src/context/internal.rs | 8 ++++++-- crates/neon/src/context/mod.rs | 28 ++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/crates/neon/src/context/internal.rs b/crates/neon/src/context/internal.rs index bf872d2e5..52164a5d6 100644 --- a/crates/neon/src/context/internal.rs +++ b/crates/neon/src/context/internal.rs @@ -1,7 +1,7 @@ use std::{cell::RefCell, ffi::c_void, mem::MaybeUninit}; use crate::{ - context::ModuleContext, + context::{Cx, ModuleContext}, handle::Handle, result::NeonResult, sys::{self, raw}, @@ -47,7 +47,11 @@ impl Env { } pub trait ContextInternal<'cx>: Sized { - fn env(&self) -> Env; + fn cx(&self) -> &Cx<'cx>; + fn cx_mut(&mut self) -> &mut Cx<'cx>; + fn env(&self) -> Env { + self.cx().env + } } fn default_main(mut cx: ModuleContext) -> NeonResult<()> { diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs index 036d65589..7bad93f4e 100644 --- a/crates/neon/src/context/mod.rs +++ b/crates/neon/src/context/mod.rs @@ -266,8 +266,12 @@ impl<'cx> Cx<'cx> { } impl<'cx> ContextInternal<'cx> for Cx<'cx> { - fn env(&self) -> Env { - self.env + fn cx(&self) -> &Cx<'cx> { + self + } + + fn cx_mut(&mut self) -> &mut Cx<'cx> { + self } } @@ -664,13 +668,13 @@ impl<'cx> Deref for ModuleContext<'cx> { type Target = Cx<'cx>; fn deref(&self) -> &Self::Target { - &self.cx + self.cx() } } impl<'cx> DerefMut for ModuleContext<'cx> { fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.cx + self.cx_mut() } } @@ -727,8 +731,12 @@ impl<'cx> ModuleContext<'cx> { } impl<'cx> ContextInternal<'cx> for ModuleContext<'cx> { - fn env(&self) -> Env { - self.cx.env + fn cx(&self) -> &Cx<'cx> { + &self.cx + } + + fn cx_mut(&mut self) -> &mut Cx<'cx> { + &mut self.cx } } @@ -873,8 +881,12 @@ impl<'cx> FunctionContext<'cx> { } impl<'cx> ContextInternal<'cx> for FunctionContext<'cx> { - fn env(&self) -> Env { - self.cx.env + fn cx(&self) -> &Cx<'cx> { + &self.cx + } + + fn cx_mut(&mut self) -> &mut Cx<'cx> { + &mut self.cx } }