refactor(ops): op2 supports Result in slow call path (#19602)

core/lib.rs

@@ -147,11 +147,6 @@ pub mod _ops {
   pub use super::runtime::V8_WRAPPER_TYPE_INDEX;
 }
 
-pub(crate) mod deno_core {
-  pub(crate) use crate::_ops;
-  pub(crate) use crate::v8;
-}
-
 // TODO(mmastrac): Temporary while we move code around
 pub mod snapshot_util {
   pub use crate::runtime::create_snapshot;

core/runtime/ops.rs

@@ -208,12 +208,25 @@ pub fn to_i64(number: &v8::Value) -> i32 {
 
 #[cfg(test)]
 mod tests {
+  use crate::error::generic_error;
+  use crate::error::AnyError;
+  use crate::error::JsError;
   use crate::FastString;
   use crate::JsRuntime;
   use crate::RuntimeOptions;
   use deno_ops::op2;
 
-  crate::extension!(testing, ops = [op_test_add, op_test_add_option]);
+  crate::extension!(
+    testing,
+    ops = [
+      op_test_add,
+      op_test_add_option,
+      op_test_result_void_ok,
+      op_test_result_void_err,
+      op_test_result_primitive_ok,
+      op_test_result_primitive_err
+    ]
+  );
 
   /// Run a test for a single op.
   fn run_test(
@@ -281,4 +294,61 @@ mod tests {
     );
     Ok(())
   }
+
+  #[op2(core)]
+  pub fn op_test_result_void_err() -> Result<(), AnyError> {
+    Err(generic_error("failed!!!"))
+  }
+
+  #[op2(core)]
+  pub fn op_test_result_void_ok() -> Result<(), AnyError> {
+    Ok(())
+  }
+
+  #[tokio::test]
+  pub async fn test_op_result_void() -> Result<(), Box<dyn std::error::Error>> {
+    run_test(
+      "op_test_result_void_err",
+      "op_test_result_void_err()",
+      |value, _scope| {
+        let js_error = value.err().unwrap().downcast::<JsError>().unwrap();
+        assert_eq!(js_error.message, Some("failed!!!".to_owned()));
+      },
+    );
+    run_test(
+      "op_test_result_void_ok",
+      "op_test_result_void_ok()",
+      |value, _scope| assert!(value.unwrap().is_null_or_undefined()),
+    );
+    Ok(())
+  }
+
+  #[op2(core)]
+  pub fn op_test_result_primitive_err() -> Result<u32, AnyError> {
+    Err(generic_error("failed!!!"))
+  }
+
+  #[op2(core)]
+  pub fn op_test_result_primitive_ok() -> Result<u32, AnyError> {
+    Ok(123)
+  }
+
+  #[tokio::test]
+  pub async fn test_op_result_primitive(
+  ) -> Result<(), Box<dyn std::error::Error>> {
+    run_test(
+      "op_test_result_primitive_err",
+      "op_test_result_primitive_err()",
+      |value, _scope| {
+        let js_error = value.err().unwrap().downcast::<JsError>().unwrap();
+        assert_eq!(js_error.message, Some("failed!!!".to_owned()));
+      },
+    );
+    run_test(
+      "op_test_result_primitive_ok",
+      "op_test_result_primitive_ok()",
+      |value, scope| assert_eq!(value.unwrap().int32_value(scope), Some(123)),
+    );
+    Ok(())
+  }
 }

ops/op2/dispatch_fast.rs

@@ -95,6 +95,11 @@ pub fn generate_dispatch_fast(
   generator_state: &mut GeneratorState,
   signature: &ParsedSignature,
 ) -> Result<Option<(TokenStream, TokenStream)>, V8MappingError> {
+  // Result not fast-call compatible (yet)
+  if matches!(signature.ret_val, RetVal::Result(..)) {
+    return Ok(None);
+  }
+
   let mut inputs = vec![];
   for arg in &signature.args {
     let fv = match arg {

ops/op2/dispatch_slow.rs

@@ -18,37 +18,34 @@ pub fn generate_dispatch_slow(
     output.extend(extract_arg(generator_state, index)?);
     output.extend(from_arg(generator_state, index, arg)?);
   }
-  output.extend(call(generator_state));
-  output.extend(return_value(generator_state, &signature.ret_val));
-
-  let GeneratorState {
-    deno_core,
-    scope,
-    fn_args,
-    retval,
-    info,
-    slow_function,
-    ..
-  } = &generator_state;
+  output.extend(call(generator_state)?);
+  output.extend(return_value(generator_state, &signature.ret_val)?);
 
   let with_scope = if generator_state.needs_scope {
-    quote!(let #scope = &mut unsafe { #deno_core::v8::CallbackScope::new(&*#info) };)
+    with_scope(generator_state)
   } else {
     quote!()
   };
 
   let with_retval = if generator_state.needs_retval {
-    quote!(let mut #retval = #deno_core::v8::ReturnValue::from_function_callback_info(unsafe { &*#info });)
+    with_retval(generator_state)
   } else {
     quote!()
   };
 
   let with_args = if generator_state.needs_args {
-    quote!(let #fn_args = #deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe { &*#info });)
+    with_fn_args(generator_state)
   } else {
     quote!()
   };
 
+  let GeneratorState {
+    deno_core,
+    info,
+    slow_function,
+    ..
+  } = &generator_state;
+
   Ok(quote! {
     pub extern "C" fn #slow_function(#info: *const #deno_core::v8::FunctionCallbackInfo) {
     #with_scope
@@ -59,6 +56,53 @@ pub fn generate_dispatch_slow(
   }})
 }
 
+fn with_scope(generator_state: &mut GeneratorState) -> TokenStream {
+  let GeneratorState {
+    deno_core,
+    scope,
+    info,
+    ..
+  } = &generator_state;
+
+  quote!(let #scope = &mut unsafe { #deno_core::v8::CallbackScope::new(&*#info) };)
+}
+
+fn with_retval(generator_state: &mut GeneratorState) -> TokenStream {
+  let GeneratorState {
+    deno_core,
+    retval,
+    info,
+    ..
+  } = &generator_state;
+
+  quote!(let mut #retval = #deno_core::v8::ReturnValue::from_function_callback_info(unsafe { &*#info });)
+}
+
+fn with_fn_args(generator_state: &mut GeneratorState) -> TokenStream {
+  let GeneratorState {
+    deno_core,
+    fn_args,
+    info,
+    ..
+  } = &generator_state;
+
+  quote!(let #fn_args = #deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe { &*#info });)
+}
+
+fn with_opctx(generator_state: &mut GeneratorState) -> TokenStream {
+  let GeneratorState {
+    deno_core,
+    opctx,
+    fn_args,
+    ..
+  } = &generator_state;
+
+  quote!(let #opctx = unsafe {
+    &*(#deno_core::v8::Local::<#deno_core::v8::External>::cast(#fn_args.data()).value()
+        as *const #deno_core::_ops::OpCtx)
+  };)
+}
+
 pub fn extract_arg(
   generator_state: &mut GeneratorState,
   index: usize,
@@ -176,6 +220,9 @@ pub fn return_value_infallible(
   } = generator_state;
 
   let res = match ret_type {
+    Arg::Void => {
+      quote! {/* void */}
+    }
     Arg::Numeric(NumericArg::u8)
     | Arg::Numeric(NumericArg::u16)
     | Arg::Numeric(NumericArg::u32) => {
@@ -204,17 +251,51 @@ pub fn return_value_result(
   ret_type: &Arg,
 ) -> Result<TokenStream, V8MappingError> {
   let infallible = return_value_infallible(generator_state, ret_type)?;
-  let GeneratorState { result, .. } = &generator_state;
+  let maybe_scope = if generator_state.needs_scope {
+    quote!()
+  } else {
+    with_scope(generator_state)
+  };
+
+  let maybe_args = if generator_state.needs_args {
+    quote!()
+  } else {
+    with_fn_args(generator_state)
+  };
+
+  let maybe_opctx = if generator_state.needs_opctx {
+    quote!()
+  } else {
+    with_opctx(generator_state)
+  };
+
+  let GeneratorState {
+    deno_core,
+    result,
+    scope,
+    opctx,
+    ..
+  } = &generator_state;
 
   let tokens = quote!(
-    let result = match ret_type {
+    match #result {
       Ok(#result) => {
-        #infallible,
+        #infallible
       }
       Err(err) => {
+        #maybe_scope
+        #maybe_args
+        #maybe_opctx
+        let opstate = ::std::cell::RefCell::borrow(&*#opctx.state);
+        let exception = #deno_core::error::to_v8_error(
+          #scope,
+          opstate.get_error_class_fn,
+          &err,
+        );
+        scope.throw_exception(exception);
         return;
       }
-    }
+    };
   );
   Ok(tokens)
 }

ops/op2/generator_state.rs

@@ -19,6 +19,8 @@ pub struct GeneratorState {
   pub info: Ident,
   /// The `v8::FunctionCallbackArguments` used to pass args into the slow function.
   pub fn_args: Ident,
+  /// The `OpCtx` used for various information required for some ops.
+  pub opctx: Ident,
   /// The `v8::ReturnValue` used in the slow function
   pub retval: Ident,
   /// The "slow" function (ie: the one that isn't a fastcall)
@@ -29,4 +31,6 @@ pub struct GeneratorState {
   pub needs_args: bool,
   pub needs_retval: bool,
   pub needs_scope: bool,
+  pub needs_opstate: bool,
+  pub needs_opctx: bool,
 }

ops/op2/mod.rs

@@ -132,11 +132,12 @@ fn generate_op2(
   let fn_args = Ident::new("args", Span::call_site());
   let scope = Ident::new("scope", Span::call_site());
   let info = Ident::new("info", Span::call_site());
+  let opctx = Ident::new("opctx", Span::call_site());
   let slow_function = Ident::new("slow_function", Span::call_site());
   let fast_function = Ident::new("fast_function", Span::call_site());
 
   let deno_core = if config.core {
-    syn2::parse_str::<Path>("crate::deno_core")
+    syn2::parse_str::<Path>("crate")
   } else {
     syn2::parse_str::<Path>("deno_core")
   }
@@ -149,6 +150,7 @@ fn generate_op2(
     call,
     scope,
     info,
+    opctx,
     deno_core,
     result,
     retval,
@@ -157,6 +159,8 @@ fn generate_op2(
     fast_function,
     needs_retval: false,
     needs_scope: false,
+    needs_opctx: false,
+    needs_opstate: false,
   };
 
   let name = func.sig.ident;

ops/op2/test_cases/sync/add_options.out

@@ -4,8 +4,8 @@ impl op_test_add_option {
     pub const fn name() -> &'static str {
         stringify!(op_test_add_option)
     }
-    pub const fn decl() -> crate::deno_core::_ops::OpDecl {
-        crate::deno_core::_ops::OpDecl {
+    pub const fn decl() -> crate::_ops::OpDecl {
+        crate::_ops::OpDecl {
             name: stringify!(op_test_add_option),
             v8_fn_ptr: Self::slow_function as _,
             enabled: true,
@@ -16,22 +16,20 @@ impl op_test_add_option {
             arg_count: 2usize as u8,
         }
     }
-    pub extern "C" fn slow_function(
-        info: *const crate::deno_core::v8::FunctionCallbackInfo,
-    ) {
-        let mut rv = crate::deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+    pub extern "C" fn slow_function(info: *const crate::v8::FunctionCallbackInfo) {
+        let mut rv = crate::v8::ReturnValue::from_function_callback_info(unsafe {
             &*info
         });
-        let args = crate::deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+        let args = crate::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
         let arg0 = args.get(0usize as i32);
-        let arg0 = crate::deno_core::_ops::to_u32(&arg0) as _;
+        let arg0 = crate::_ops::to_u32(&arg0) as _;
         let arg1 = args.get(1usize as i32);
         let arg1 = if arg1.is_null_or_undefined() {
             None
         } else {
-            let arg1 = crate::deno_core::_ops::to_u32(&arg1) as _;
+            let arg1 = crate::_ops::to_u32(&arg1) as _;
             Some(arg1)
         };
         let result = Self::call(arg0, arg1);

ops/op2/test_cases/sync/result_primitive.out

@@ -0,0 +1,50 @@
+#[allow(non_camel_case_types)]
+pub struct op_u32_with_result {}
+impl op_u32_with_result {
+    pub const fn name() -> &'static str {
+        stringify!(op_u32_with_result)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_u32_with_result),
+            v8_fn_ptr: Self::slow_function as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 0usize as u8,
+        }
+    }
+    pub extern "C" fn slow_function(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
+        let result = Self::call();
+        match result {
+            Ok(result) => {
+                rv.set_uint32(result as u32);
+            }
+            Err(err) => {
+                let scope = &mut unsafe { deno_core::v8::CallbackScope::new(&*info) };
+                let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+                    &*info
+                });
+                let opctx = unsafe {
+                    &*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data())
+                        .value() as *const deno_core::_ops::OpCtx)
+                };
+                let opstate = ::std::cell::RefCell::borrow(&*opctx.state);
+                let exception = deno_core::error::to_v8_error(
+                    scope,
+                    opstate.get_error_class_fn,
+                    &err,
+                );
+                scope.throw_exception(exception);
+                return;
+            }
+        };
+    }
+    #[inline(always)]
+    pub fn call() -> Result<u32, AnyError> {}
+}

ops/op2/test_cases/sync/result_primitive.rs

@@ -0,0 +1,4 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+pub fn op_u32_with_result() -> Result<u32, AnyError> {}

ops/op2/test_cases/sync/result_void.out

@@ -0,0 +1,45 @@
+#[allow(non_camel_case_types)]
+pub struct op_void_with_result {}
+impl op_void_with_result {
+    pub const fn name() -> &'static str {
+        stringify!(op_void_with_result)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_void_with_result),
+            v8_fn_ptr: Self::slow_function as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 0usize as u8,
+        }
+    }
+    pub extern "C" fn slow_function(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let result = Self::call();
+        match result {
+            Ok(result) => {}
+            Err(err) => {
+                let scope = &mut unsafe { deno_core::v8::CallbackScope::new(&*info) };
+                let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+                    &*info
+                });
+                let opctx = unsafe {
+                    &*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data())
+                        .value() as *const deno_core::_ops::OpCtx)
+                };
+                let opstate = ::std::cell::RefCell::borrow(&*opctx.state);
+                let exception = deno_core::error::to_v8_error(
+                    scope,
+                    opstate.get_error_class_fn,
+                    &err,
+                );
+                scope.throw_exception(exception);
+                return;
+            }
+        };
+    }
+    #[inline(always)]
+    pub fn call() -> Result<(), AnyError> {}
+}

ops/op2/test_cases/sync/result_void.rs

@@ -0,0 +1,4 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+pub fn op_void_with_result() -> Result<(), AnyError> {}