Repository: denoland/deno_core (rust)

Issue:
`op2` should allow `where` clause with `'static`
`op2` supports `where T: Trait` , but it does not support `where T: 'static`. This should be considered valid as there may be cases where the op requires a `'static` trait but the trait does not need to be `'static`.

Pull request description:
fix(ops): support where T: Trait + 'static in generics
Fixes #140


Patch:
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index e6d5afd..ae4e17b 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -767,6 +767,7 @@ mod tests {
       op_async_buffer_vec,
       op_async_buffer_impl,
       op_async_external,
+      op_async_serde_option_v8,
     ],
     state = |state| {
       state.put(1234u32);
@@ -1940,4 +1941,25 @@ mod tests {
     .await?;
     Ok(())
   }
+
+  #[op2(async, core)]
+  #[serde]
+  pub async fn op_async_serde_option_v8(
+    #[serde] mut serde: Serde,
+  ) -> Result<Option<Serde>, AnyError> {
+    serde.s += "!";
+    Ok(Some(serde))
+  }
+
+  #[tokio::test]
+  pub async fn test_op_async_serde_option_v8(
+  ) -> Result<(), Box<dyn std::error::Error>> {
+    run_async_test(
+      2,
+      "op_async_serde_option_v8",
+      "assert((await op_async_serde_option_v8({s: 'abc'})).s == 'abc!')",
+    )
+    .await?;
+    Ok(())
+  }
 }
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index c38515f..154f46d 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -681,6 +681,9 @@ pub fn return_value_v8_value(
     Arg::External(External::Ptr(_)) => {
       quote!(Ok(#deno_core::v8::External::new(#scope, #result as _).into()))
     }
+    Arg::SerdeV8(_) => {
+      quote!(#deno_core::_ops::serde_rust_to_v8(#scope, #result))
+    }
     _ => {
       return Err(V8MappingError::NoMapping(
         "a v8 return value",
diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs
index 01d341d..a948ba7 100644
--- a/ops/op2/mod.rs
+++ b/ops/op2/mod.rs
@@ -14,6 +14,7 @@ use syn::ItemFn;
 use syn::Lifetime;
 use syn::LifetimeParam;
 use syn::Path;
+use syn::Type;
 use thiserror::Error;
 
 use self::config::MacroConfig;
@@ -223,7 +224,7 @@ fn generate_op2(
   let bound = signature
     .generic_bounds
     .values()
-    .map(|p| parse_str::<Path>(p).expect("Failed to reparse path"))
+    .map(|p| parse_str::<Type>(p).expect("Failed to reparse type bounds"))
     .collect::<Vec<_>>();
 
   Ok(quote! {
diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs
index 72ede2b..b8064ee 100644
--- a/ops/op2/signature.rs
+++ b/ops/op2/signature.rs
@@ -20,6 +20,7 @@ use syn::Pat;
 use syn::Path;
 use syn::Signature;
 use syn::Type;
+use syn::TypeParamBound;
 use syn::TypePath;
 use thiserror::Error;
 
@@ -710,6 +711,52 @@ fn parse_lifetime(
   Ok(res)
 }
 
+/// Parse a bound as a string. Valid bounds include "Trait" and "Trait + 'static". All
+/// other bounds are invalid.
+fn parse_bound(bound: &Type) -> Result<String, SignatureError> {
+  let error = || {
+    Err(SignatureError::InvalidWherePredicate(stringify_token(
+      bound,
+    )))
+  };
+
+  Ok(match bound {
+    Type::TraitObject(t) => {
+      let mut has_static_lifetime = false;
+      let mut bound = None;
+      for b in &t.bounds {
+        match b {
+          TypeParamBound::Lifetime(lt) => {
+            if lt.ident != "static" || has_static_lifetime {
+              return error();
+            }
+            has_static_lifetime = true;
+          }
+          TypeParamBound::Trait(t) => {
+            if bound.is_some() {
+              return error();
+            }
+            bound = Some(stringify_token(t));
+          }
+          _ => return error(),
+        }
+      }
+      let Some(bound) = bound else {
+        return error();
+      };
+      if has_static_lifetime {
+        format!("{bound} + 'static")
+      } else {
+        bound
+      }
+    }
+    Type::Path(p) => stringify_token(p),
+    _ => {
+      return error();
+    }
+  })
+}
+
 /// Parse and validate generics. We require one and only one trait bound for each generic
 /// parameter. Tries to sanity check and return reasonable errors for possible signature errors.
 fn parse_generics(
@@ -723,12 +770,13 @@ fn parse_generics(
       let predicate = predicate.to_token_stream();
       let (generic_name, bound) = std::panic::catch_unwind(|| {
         rules!(predicate => {
-          ($t:ident : $bound:path) => (t.to_string(), stringify_token(bound)),
+          ($t:ident : $bound:ty) => (t.to_string(), bound),
         })
       })
       .map_err(|_| {
         SignatureError::InvalidWherePredicate(predicate.to_string())
       })?;
+      let bound = parse_bound(&bound)?;
       if where_clauses.insert(generic_name.clone(), bound).is_some() {
         return Err(SignatureError::DuplicateGeneric(generic_name));
       }
@@ -741,16 +789,17 @@ fn parse_generics(
       let ty = ty.to_token_stream();
       let (name, bound) = std::panic::catch_unwind(|| {
         rules!(ty => {
-          ($t:ident : $bound:path) => (t.to_string(), Some(stringify_token(bound))),
+          ($t:ident : $bound:ty) => (t.to_string(), Some(bound)),
           ($t:ident) => (t.to_string(), None),
         })
-      }).map_err(|_| SignatureError::InvalidGeneric(ty.to_string()))?;
+      })
+      .map_err(|_| SignatureError::InvalidGeneric(ty.to_string()))?;
       let bound = match bound {
         Some(bound) => {
           if where_clauses.contains_key(&name) {
             return Err(SignatureError::GenericBoundCardinality(name));
           }
-          bound
+          parse_bound(&bound)?
         }
         None => {
           let Some(bound) = where_clauses.remove(&name) else {
@@ -1177,10 +1226,10 @@ mod tests {
       // Return value
       $(-> $(# [ $ret_attr:meta ])? $ret:ty)?
       // Where clause
-      $( where $($trait:ident : $bounds:path),* )?
+      $( where $($trait:ident : $bounds:ty),* )?
       ;
       // Expected return value
-      $( < $( $lifetime_res:lifetime )? $(, $generic_res:ident : $bounds_res:path )* >)? ( $( $arg_res:expr ),* ) -> $ret_res:expr ) => {
+      $( < $( $lifetime_res:lifetime )? $(, $generic_res:ident : $bounds_res:ty )* >)? ( $( $arg_res:expr ),* ) -> $ret_res:expr ) => {
       #[test]
       fn $($name1)? $($name2)? () {
         test(
@@ -1228,10 +1277,15 @@ mod tests {
         .trim_matches(|c| c == '[' || c == ']')
         .replace('\n', " ")
         .replace('"', "")
+        // Use the turbofish syntax (ugly but it's just for tests)
+        .replace('<', "::<")
     );
     assert_eq!(
       return_expected,
-      format!("{:?}", sig.ret_val).replace('"', "")
+      format!("{:?}", sig.ret_val)
+        .replace('"', "")
+        // Use the turbofish syntax (ugly but it's just for tests)
+        .replace('<', "::<")
     );
   }
 
@@ -1266,6 +1320,11 @@ mod tests {
     #[serde] fn op_serde(#[serde] input: package::SerdeInputType) -> Result<package::SerdeReturnType, Error>;
     (SerdeV8(package::SerdeInputType)) -> Result(SerdeV8(package::SerdeReturnType))
   );
+  // Note the turbofish syntax here because of macro constraints
+  test!(
+    #[serde] fn op_serde_option(#[serde] maybe: Option<package::SerdeInputType>) -> Result<Option<package::SerdeReturnType>, Error>;
+    (SerdeV8(Option::<package::SerdeInputType>)) -> Result(SerdeV8(Option::<package::SerdeReturnType>))
+  );
   test!(
     #[serde] fn op_serde_tuple(#[serde] input: (A, B)) -> (A, B);
     (SerdeV8((A, B))) -> Infallible(SerdeV8((A, B)))
@@ -1314,6 +1373,10 @@ mod tests {
     fn op_scope_and_generics<'s, AB, BC>(#[string] msg: &'s str) where AB: some::Trait, BC: OtherTrait;
     <'s, AB: some::Trait, BC: OtherTrait> (String(RefStr)) -> Infallible(Void)
   );
+  test!(
+    fn op_generics_static<'s, AB, BC>(#[string] msg: &'s str) where AB: some::Trait + 'static, BC: OtherTrait;
+    <'s, AB: some::Trait + 'static, BC: OtherTrait> (String(RefStr)) -> Infallible(Void)
+  );
   test!(
     fn op_v8_types(s: &mut v8::String, sopt: Option<&mut v8::String>, s2: v8::Local<v8::String>, #[global] s3: v8::Global<v8::String>);
     (V8Ref(Mut, String), OptionV8Ref(Mut, String), V8Local(String), V8Global(String)) -> Infallible(Void)


Test patch:
diff --git a/ops/op2/test_cases/sync/generics.out b/ops/op2/test_cases/sync/generics.out
index 27431e0..eb267dd 100644
--- a/ops/op2/test_cases/sync/generics.out
+++ b/ops/op2/test_cases/sync/generics.out
@@ -44,3 +44,100 @@ impl<T: Trait> op_generics<T> {
     #[inline(always)]
     pub fn call() {}
 }
+
+#[allow(non_camel_case_types)]
+pub struct op_generics_static<T> {
+    _unconstructable: ::std::marker::PhantomData<(T)>,
+}
+impl<T: Trait + 'static> deno_core::_ops::Op for op_generics_static<T> {
+    const NAME: &'static str = stringify!(op_generics_static);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl::new_internal(
+        stringify!(op_generics_static),
+        false,
+        false,
+        false,
+        0usize as u8,
+        Self::v8_fn_ptr as _,
+        Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new_with_bigint(
+                &[Type::V8Value],
+                CType::Void,
+                Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+            )
+        }),
+    );
+}
+impl<T: Trait + 'static> op_generics_static<T> {
+    pub const fn name() -> &'static str {
+        stringify!(op_generics_static)
+    }
+    #[deprecated(note = "Use the const op::DECL instead")]
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        <Self as deno_core::_ops::Op>::DECL
+    }
+    fn v8_fn_ptr_fast(_: deno_core::v8::Local<deno_core::v8::Object>) -> () {
+        let result = { Self::call() };
+        result as _
+    }
+    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() {}
+}
+
+#[allow(non_camel_case_types)]
+pub struct op_generics_static_where<T> {
+    _unconstructable: ::std::marker::PhantomData<(T)>,
+}
+impl<T: Trait + 'static> deno_core::_ops::Op for op_generics_static_where<T> {
+    const NAME: &'static str = stringify!(op_generics_static_where);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl::new_internal(
+        stringify!(op_generics_static_where),
+        false,
+        false,
+        false,
+        0usize as u8,
+        Self::v8_fn_ptr as _,
+        Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new_with_bigint(
+                &[Type::V8Value],
+                CType::Void,
+                Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+            )
+        }),
+    );
+}
+impl<T: Trait + 'static> op_generics_static_where<T> {
+    pub const fn name() -> &'static str {
+        stringify!(op_generics_static_where)
+    }
+    #[deprecated(note = "Use the const op::DECL instead")]
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        <Self as deno_core::_ops::Op>::DECL
+    }
+    fn v8_fn_ptr_fast(_: deno_core::v8::Local<deno_core::v8::Object>) -> () {
+        let result = { Self::call() };
+        result as _
+    }
+    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()
+    where
+        T: Trait + 'static,
+    {}
+}
diff --git a/ops/op2/test_cases/sync/generics.rs b/ops/op2/test_cases/sync/generics.rs
index ee6cdc0..e6ada46 100644
--- a/ops/op2/test_cases/sync/generics.rs
+++ b/ops/op2/test_cases/sync/generics.rs
@@ -6,3 +6,9 @@ pub trait Trait {}
 
 #[op2(fast)]
 pub fn op_generics<T: Trait>() {}
+
+#[op2(fast)]
+pub fn op_generics_static<T: Trait + 'static>() {}
+
+#[op2(fast)]
+pub fn op_generics_static_where<T>() where T: Trait + 'static {}