Repository: denoland/deno_core
Pull request #55 (2023-07-19 16:34:04)

fix(ops): set return value to null for () Rust return


Patch:
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 7176d4d..68616a6 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -482,7 +482,12 @@ pub fn return_value_infallible(
 
   let res = match ret_type {
     Arg::Void => {
-      quote! {/* void */}
+      // TODO(mmastrac): revisit this. Ideally we wouldn't need to set
+      // rv to null, but because of how serde_v8 works this is required
+      // to keep compatibility with existing assumptions in `deno_core`
+      // and `deno` itself.
+      *needs_retval = true;
+      quote! {#retval.set_null();}
     }
     Arg::Numeric(NumericArg::bool) => {
       *needs_retval = true;


Test patch:
diff --git a/ops/op2/test_cases/async/async_void.out b/ops/op2/test_cases/async/async_void.out
index 75924af..c3af98c 100644
--- a/ops/op2/test_cases/async/async_void.out
+++ b/ops/op2/test_cases/async/async_void.out
@@ -42,7 +42,9 @@ impl op_async {
                 promise_id,
                 result,
                 |scope, result| { Ok(deno_core::v8::null(scope).into()) },
-            ) {}
+            ) {
+            rv.set_null();
+        }
     }
     #[inline(always)]
     pub async fn call() {}
diff --git a/ops/op2/test_cases/sync/buffers.out b/ops/op2/test_cases/sync/buffers.out
index 904711c..14ffd25 100644
--- a/ops/op2/test_cases/sync/buffers.out
+++ b/ops/op2/test_cases/sync/buffers.out
@@ -46,6 +46,9 @@ impl op_buffers {
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
         let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -87,6 +90,7 @@ impl op_buffers {
         };
         let arg1 = &mut arg1;
         let result = Self::call(arg0, arg1);
+        rv.set_null();
     }
     #[inline(always)]
     fn call(_a: &[u8], _b: &mut [u8]) {}
diff --git a/ops/op2/test_cases/sync/buffers_copy.out b/ops/op2/test_cases/sync/buffers_copy.out
index 1e1d35f..f061cda 100644
--- a/ops/op2/test_cases/sync/buffers_copy.out
+++ b/ops/op2/test_cases/sync/buffers_copy.out
@@ -60,6 +60,9 @@ impl op_buffers {
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
         let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -119,6 +122,7 @@ impl op_buffers {
         };
         let arg2 = arg2.to_vec().into();
         let result = Self::call(arg0, arg1, arg2);
+        rv.set_null();
     }
     #[inline(always)]
     fn call(_a: Vec<u8>, _b: Box<[u8]>, _c: bytes::Bytes) {}
diff --git a/ops/op2/test_cases/sync/clippy_allow.out b/ops/op2/test_cases/sync/clippy_allow.out
index cd83c38..9c187a3 100644
--- a/ops/op2/test_cases/sync/clippy_allow.out
+++ b/ops/op2/test_cases/sync/clippy_allow.out
@@ -37,7 +37,11 @@ impl op_extra_annotation {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let result = Self::call();
+        rv.set_null();
     }
     #[inline(always)]
     /// This is a doc comment.
diff --git a/ops/op2/test_cases/sync/doc_comment.out b/ops/op2/test_cases/sync/doc_comment.out
index b3647c2..fd04f34 100644
--- a/ops/op2/test_cases/sync/doc_comment.out
+++ b/ops/op2/test_cases/sync/doc_comment.out
@@ -36,7 +36,11 @@ impl op_has_doc_comment {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let result = Self::call();
+        rv.set_null();
     }
     #[inline(always)]
     /// This is a doc comment.
diff --git a/ops/op2/test_cases/sync/generics.out b/ops/op2/test_cases/sync/generics.out
index 6ab8aa9..4b0492d 100644
--- a/ops/op2/test_cases/sync/generics.out
+++ b/ops/op2/test_cases/sync/generics.out
@@ -35,7 +35,11 @@ impl<T: Trait> op_generics<T> {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let result = Self::call();
+        rv.set_null();
     }
     #[inline(always)]
     pub fn call() {}
diff --git a/ops/op2/test_cases/sync/op_state_attr.out b/ops/op2/test_cases/sync/op_state_attr.out
index 43a3347..92a27dd 100644
--- a/ops/op2/test_cases/sync/op_state_attr.out
+++ b/ops/op2/test_cases/sync/op_state_attr.out
@@ -49,6 +49,9 @@ impl op_state_rc {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -62,6 +65,7 @@ impl op_state_rc {
         let arg1 = opstate.borrow();
         let arg1 = arg1.try_borrow::<Something>();
         let result = Self::call(arg0, arg1);
+        rv.set_null();
     }
     #[inline(always)]
     fn call(_arg: &Something, _arg_opt: Option<&Something>) {}
diff --git a/ops/op2/test_cases/sync/op_state_rc.out b/ops/op2/test_cases/sync/op_state_rc.out
index 59b3f6a..03685f2 100644
--- a/ops/op2/test_cases/sync/op_state_rc.out
+++ b/ops/op2/test_cases/sync/op_state_rc.out
@@ -46,6 +46,9 @@ impl op_state_rc {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -56,6 +59,7 @@ impl op_state_rc {
         let opstate = &opctx.state;
         let arg0 = opstate.clone();
         let result = Self::call(arg0);
+        rv.set_null();
     }
     #[inline(always)]
     fn call(_state: Rc<RefCell<OpState>>) {}
diff --git a/ops/op2/test_cases/sync/op_state_ref.out b/ops/op2/test_cases/sync/op_state_ref.out
index 4db27b7..0872a20 100644
--- a/ops/op2/test_cases/sync/op_state_ref.out
+++ b/ops/op2/test_cases/sync/op_state_ref.out
@@ -46,6 +46,9 @@ impl op_state_rc {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -56,6 +59,7 @@ impl op_state_rc {
         let opstate = &opctx.state;
         let arg0 = &opstate.borrow();
         let result = Self::call(arg0);
+        rv.set_null();
     }
     #[inline(always)]
     fn call(_state: &OpState) {}
diff --git a/ops/op2/test_cases/sync/result_scope.out b/ops/op2/test_cases/sync/result_scope.out
index 4d28ad8..e5069e3 100644
--- a/ops/op2/test_cases/sync/result_scope.out
+++ b/ops/op2/test_cases/sync/result_scope.out
@@ -24,13 +24,18 @@ impl op_void_with_result {
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
         let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
         let arg0 = &mut scope;
         let result = Self::call(arg0);
         match result {
-            Ok(result) => {}
+            Ok(result) => {
+                rv.set_null();
+            }
             Err(err) => {
                 let opctx = unsafe {
                     &*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data())
diff --git a/ops/op2/test_cases/sync/result_void.out b/ops/op2/test_cases/sync/result_void.out
index eac8a27..10af48e 100644
--- a/ops/op2/test_cases/sync/result_void.out
+++ b/ops/op2/test_cases/sync/result_void.out
@@ -55,6 +55,9 @@ impl op_void_with_result {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -79,7 +82,9 @@ impl op_void_with_result {
         }
         let result = Self::call();
         match result {
-            Ok(result) => {}
+            Ok(result) => {
+                rv.set_null();
+            }
             Err(err) => {
                 let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
                 let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
diff --git a/ops/op2/test_cases/sync/v8_ref_option.out b/ops/op2/test_cases/sync/v8_ref_option.out
index a78b080..57ba6bb 100644
--- a/ops/op2/test_cases/sync/v8_ref_option.out
+++ b/ops/op2/test_cases/sync/v8_ref_option.out
@@ -59,6 +59,9 @@ impl op_v8_lifetime {
         result
     }
     extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
         let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
             &*info
         });
@@ -101,6 +104,7 @@ impl op_v8_lifetime {
             Some(v) => Some(::std::ops::Deref::deref(v)),
         };
         let result = Self::call(arg0, arg1);
+        rv.set_null();
     }
     #[inline(always)]
     pub fn call<'s>(_s: Option<&v8::String>, _s2: Option<&v8::String>) {}