Repository: denoland/deno_core
Pull request #12 (2023-07-03 19:02:22)

feat(core): Implement v8::XXX args and return
`op2` support for `v8::Value` and related types. Functions may take arguments of the form `&v8::String` (or `&mut v8::String`), `v8::Local<v8::String>`, or an `Option<...>` of any of these. Under the hood, the op system will check the type of the argument and automatically throw a type error if it does not match.

Implements:

 - [x] v8::Local<v8::XXX> for arguments (including automatic casting so `v8::String` will work)
 - [x] v8::Local<v8::XXX> for return types
 - [x] Option<v8::Local<...>> for arguments and returns
 - [x] &v8::XXX and Option<&v8:XXX> for arguments (ie: pass a `&mut v8::String` without a handle)
 - [x] HandleScope

Additional changes:

 - [x] Fixes clippy attributes on ops
 - [x] Refactoring of signature parser to be slightly less tied to our final arg/return types
 - [x] Ensures that lifetimes are properly added to `call`




CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/denoland/deno_core?pullRequest=12) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/denoland/deno_core?pullRequest=12) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/denoland/deno_core?pullRequest=12) it.</sub>

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 37fba2e..b9d4253 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1021,12 +1021,12 @@ dependencies = [
 
 [[package]]
 name = "prettyplease"
-version = "0.1.25"
+version = "0.2.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6c8646e95016a7a6c4adea95bafa8a16baab64b583356217f2c85db4a39d9a86"
+checksum = "9825a04601d60621feed79c4e6b56d65db77cdca55cef43b46b0de1096d1c282"
 dependencies = [
  "proc-macro2",
- "syn 1.0.109",
+ "syn 2.0.18",
 ]
 
 [[package]]
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index 5ecab5e..a02750d 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -328,6 +328,12 @@ mod tests {
       op_test_string_option_return,
       op_test_string_roundtrip,
       op_test_generics<String>,
+      op_test_v8_types,
+      op_test_v8_option_string,
+      op_test_v8_type_return,
+      op_test_v8_type_return_option,
+      op_test_v8_type_handle_scope,
+      op_test_v8_type_handle_scope_obj,
     ]
   );
 
@@ -631,4 +637,100 @@ mod tests {
   // We don't actually test this one -- we just want it to compile
   #[op2(core, fast)]
   pub fn op_test_generics<T: Clone>() {}
+
+  /// Tests v8 types without a handle scope
+  #[allow(clippy::needless_lifetimes)]
+  #[op2(core)]
+  pub fn op_test_v8_types<'s>(
+    s: &v8::String,
+    s2: v8::Local<v8::String>,
+    s3: v8::Local<'s, v8::String>,
+  ) -> u32 {
+    if s.same_value(s2.into()) {
+      1
+    } else if s.same_value(s3.into()) {
+      2
+    } else {
+      3
+    }
+  }
+
+  #[op2(core)]
+  pub fn op_test_v8_option_string(s: Option<&v8::String>) -> i32 {
+    if let Some(s) = s {
+      s.length() as i32
+    } else {
+      -1
+    }
+  }
+
+  /// Tests v8 types without a handle scope
+  #[op2(core)]
+  #[allow(clippy::needless_lifetimes)]
+  pub fn op_test_v8_type_return<'s>(
+    s: v8::Local<'s, v8::String>,
+  ) -> v8::Local<'s, v8::String> {
+    s
+  }
+
+  /// Tests v8 types without a handle scope
+  #[op2(core)]
+  #[allow(clippy::needless_lifetimes)]
+  pub fn op_test_v8_type_return_option<'s>(
+    s: Option<v8::Local<'s, v8::String>>,
+  ) -> Option<v8::Local<'s, v8::String>> {
+    s
+  }
+
+  #[op2(core)]
+  pub fn op_test_v8_type_handle_scope<'s>(
+    scope: &mut v8::HandleScope<'s>,
+    s: &v8::String,
+  ) -> v8::Local<'s, v8::String> {
+    let s = s.to_rust_string_lossy(scope);
+    v8::String::new(scope, &s).unwrap()
+  }
+
+  /// Extract whatever lives in "key" from the object.
+  #[op2(core)]
+  pub fn op_test_v8_type_handle_scope_obj<'s>(
+    scope: &mut v8::HandleScope<'s>,
+    o: &v8::Object,
+  ) -> Option<v8::Local<'s, v8::Value>> {
+    let key = v8::String::new(scope, "key").unwrap().into();
+    o.get(scope, key)
+  }
+
+  #[tokio::test]
+  pub async fn test_op_v8_types() -> Result<(), Box<dyn std::error::Error>> {
+    for (a, b) in [("a", 1), ("b", 2), ("c", 3)] {
+      run_test2(
+        1,
+        "op_test_v8_types",
+        &format!("assert(op_test_v8_types('{a}', 'a', 'b') == {b})"),
+      )?;
+    }
+    for (a, b, c) in [
+      ("op_test_v8_type_return", "'xyz'", "'xyz'"),
+      ("op_test_v8_option_string", "'xyz'", "3"),
+      ("op_test_v8_option_string", "null", "-1"),
+      ("op_test_v8_type_return_option", "'xyz'", "'xyz'"),
+      ("op_test_v8_type_return_option", "null", "null"),
+      ("op_test_v8_type_handle_scope", "'xyz'", "'xyz'"),
+      ("op_test_v8_type_handle_scope_obj", "{'key': 1}", "1"),
+      (
+        "op_test_v8_type_handle_scope_obj",
+        "{'key': 'abc'}",
+        "'abc'",
+      ),
+      (
+        "op_test_v8_type_handle_scope_obj",
+        "{'no_key': 'abc'}",
+        "null",
+      ),
+    ] {
+      run_test2(1, a, &format!("assert({a}({b}) == {c})"))?;
+    }
+    Ok(())
+  }
 }
diff --git a/ops/Cargo.toml b/ops/Cargo.toml
index 29753c1..34769f3 100644
--- a/ops/Cargo.toml
+++ b/ops/Cargo.toml
@@ -32,6 +32,6 @@ v8.workspace = true
 
 [dev-dependencies]
 pretty_assertions.workspace = true
-prettyplease = "0.1.21"
+prettyplease = "0.2.9"
 testing_macros = "0.2.7"
 trybuild = "1.0.71"
diff --git a/ops/lib.rs b/ops/lib.rs
index 31398a1..a27b36f 100644
--- a/ops/lib.rs
+++ b/ops/lib.rs
@@ -1013,7 +1013,7 @@ mod tests {
     println!("-----Raw tokens-----\n{}----------\n", actual);
 
     // Validate syntax tree.
-    let tree = syn::parse2(actual).unwrap();
+    let tree = syn2::parse2(actual).unwrap();
     let actual = prettyplease::unparse(&tree);
     if update_expected {
       std::fs::write(input.with_extension("out"), actual)
diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs
index f9d7441..713ab08 100644
--- a/ops/op2/dispatch_fast.rs
+++ b/ops/op2/dispatch_fast.rs
@@ -267,7 +267,14 @@ fn map_arg_to_v8_fastcall_type(
   arg: &Arg,
 ) -> Result<Option<V8FastCallType>, V8MappingError> {
   let rv = match arg {
-    Arg::OptionNumeric(_) | Arg::SerdeV8(_) => return Ok(None),
+    Arg::OptionNumeric(_) | Arg::SerdeV8(_) | Arg::Ref(..) => return Ok(None),
+    // We don't support v8 type arguments
+    Arg::V8Ref(..)
+    | Arg::V8Global(_)
+    | Arg::V8Local(_)
+    | Arg::OptionV8Local(_)
+    | Arg::OptionV8Ref(..) => return Ok(None),
+
     Arg::Numeric(NumericArg::bool) => V8FastCallType::Bool,
     Arg::Numeric(NumericArg::u32)
     | Arg::Numeric(NumericArg::u16)
@@ -316,6 +323,12 @@ fn map_retval_to_v8_fastcall_type(
     // We don't return special return types
     Arg::Option(_) => return Ok(None),
     Arg::Special(_) => return Ok(None),
+    // We don't support returning v8 types
+    Arg::V8Ref(..)
+    | Arg::V8Global(_)
+    | Arg::V8Local(_)
+    | Arg::OptionV8Local(_)
+    | Arg::OptionV8Ref(..) => return Ok(None),
     _ => {
       return Err(V8MappingError::NoMapping(
         "a fast return value",
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 2ec67cc..4708037 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -3,10 +3,13 @@ use super::generator_state::GeneratorState;
 use super::signature::Arg;
 use super::signature::NumericArg;
 use super::signature::ParsedSignature;
+use super::signature::RefType;
 use super::signature::RetVal;
 use super::signature::Special;
+use super::signature::V8Arg;
 use super::MacroConfig;
 use super::V8MappingError;
+use proc_macro2::Ident;
 use proc_macro2::TokenStream;
 use quote::format_ident;
 use quote::quote;
@@ -36,10 +39,22 @@ pub(crate) fn generate_dispatch_slow(
     });
   }
 
+  // Collect virtual arguments in a deferred list that we compute at the very end. This allows us to copy
+  // the scope borrow.
+  let mut deferred = TokenStream::new();
+  let mut input_index = 0;
+
   for (index, arg) in signature.args.iter().enumerate() {
-    output.extend(extract_arg(generator_state, index)?);
-    output.extend(from_arg(generator_state, index, arg)?);
+    if arg.is_virtual() {
+      deferred.extend(from_arg(generator_state, index, arg)?);
+    } else {
+      output.extend(extract_arg(generator_state, index, input_index)?);
+      output.extend(from_arg(generator_state, index, arg)?);
+      input_index += 1;
+    }
   }
+
+  output.extend(deferred);
   output.extend(call(generator_state)?);
   output.extend(return_value(generator_state, &signature.ret_val)?);
 
@@ -137,12 +152,13 @@ fn with_opctx(generator_state: &mut GeneratorState) -> TokenStream {
 pub fn extract_arg(
   generator_state: &mut GeneratorState,
   index: usize,
+  input_index: usize,
 ) -> Result<TokenStream, V8MappingError> {
   let GeneratorState { fn_args, .. } = &generator_state;
   let arg_ident = generator_state.args.get(index);
 
   Ok(quote!(
-    let #arg_ident = #fn_args.get(#index as i32);
+    let #arg_ident = #fn_args.get(#input_index as i32);
   ))
 }
 
@@ -230,11 +246,85 @@ pub fn from_arg(
         let #arg_ident = #deno_core::_ops::to_str(#scope, &#arg_ident, &mut #arg_temp);
       }
     }
+    Arg::Ref(_, Special::HandleScope) => {
+      *needs_scope = true;
+      quote!(let #arg_ident = #scope;)
+    }
+    Arg::V8Ref(_, v8) => {
+      let arg_ident = arg_ident.clone();
+      let arg = from_v8_arg(generator_state, v8, &arg_ident)?;
+      quote! {
+        #arg;
+        let #arg_ident = &#arg_ident;
+      }
+    }
+    Arg::V8Local(v8) => {
+      let arg_ident = arg_ident.clone();
+      from_v8_arg(generator_state, v8, &arg_ident)?
+    }
+    Arg::OptionV8Ref(RefType::Ref, v8) => {
+      let arg_ident = arg_ident.clone();
+      let arg = from_arg(generator_state, index, &Arg::OptionV8Local(*v8))?;
+      quote! {
+        // First get the Option<Local>
+        #arg;
+        // Then map the reference
+        let #arg_ident = #arg_ident.as_ref().map(|v| ::std::ops::Deref::deref(v));
+      }
+    }
+    Arg::OptionV8Ref(RefType::Mut, v8) => {
+      let arg_ident = arg_ident.clone();
+      let arg = from_arg(generator_state, index, &Arg::OptionV8Local(*v8))?;
+      quote! {
+        // First get the Option<Local>
+        #arg;
+        // Then map the reference
+        let #arg_ident = #arg_ident.as_mut().map(|v| ::std::ops::DerefMut::deref_mut(v));
+      }
+    }
+    Arg::OptionV8Local(v8) => {
+      let arg_ident = arg_ident.clone();
+      let arg = from_v8_arg(generator_state, v8, &arg_ident)?;
+      quote! {
+        let #arg_ident = if #arg_ident.is_null_or_undefined() {
+          None
+        } else {
+          #arg;
+          Some(#arg_ident)
+        };
+      }
+    }
     _ => return Err(V8MappingError::NoMapping("a slow argument", arg.clone())),
   };
   Ok(res)
 }
 
+/// Generates a v8::Value of the correct type for the required V8Arg, throwing an exception if the
+/// type cannot be cast.
+fn from_v8_arg(
+  generator_state: &mut GeneratorState,
+  v8: &V8Arg,
+  arg_ident: &Ident,
+) -> Result<TokenStream, V8MappingError> {
+  let GeneratorState {
+    deno_core,
+    needs_scope,
+    ..
+  } = generator_state;
+
+  *needs_scope = true;
+  let v8: &'static str = v8.into();
+  let deno_core = deno_core.clone();
+  let throw_type_error =
+    throw_type_error(generator_state, format!("expected v8::{}", v8))?;
+  let v8 = format_ident!("{}", v8);
+  Ok(quote! {
+    let Ok(mut #arg_ident) = #deno_core::v8::Local::<#deno_core::v8::#v8>::try_from(#arg_ident) else {
+      #throw_type_error
+    };
+  })
+}
+
 pub fn call(
   generator_state: &mut GeneratorState,
 ) -> Result<TokenStream, V8MappingError> {
@@ -323,6 +413,24 @@ pub fn return_value_infallible(
         }
       }
     }
+    Arg::OptionV8Local(_) => {
+      *needs_retval = true;
+      quote! {
+        if let Some(#result) = #result {
+          // We may have a non v8::Value here
+          #retval.set(#result.into())
+        } else {
+          #retval.set_null();
+        }
+      }
+    }
+    Arg::V8Local(_) => {
+      *needs_retval = true;
+      quote! {
+        // We may have a non v8::Value here
+        #retval.set(#result.into())
+      }
+    }
     _ => {
       return Err(V8MappingError::NoMapping(
         "a slow return value",
@@ -395,7 +503,38 @@ fn throw_exception(
       opstate.get_error_class_fn,
       &err,
     );
-    scope.throw_exception(exception);
+    #scope.throw_exception(exception);
+    return;
+  })
+}
+
+/// Generates code to throw an exception, adding required additional dependencies as needed.
+fn throw_type_error(
+  generator_state: &mut GeneratorState,
+  message: String,
+) -> Result<TokenStream, V8MappingError> {
+  // Sanity check ASCII and a valid/reasonable message size
+  debug_assert!(
+    message.is_ascii()
+      && message.len() < v8::String::max_length()
+      && message.len() < 1024
+  );
+
+  let maybe_scope = if generator_state.needs_scope {
+    quote!()
+  } else {
+    with_scope(generator_state)
+  };
+
+  let GeneratorState {
+    deno_core, scope, ..
+  } = &generator_state;
+
+  Ok(quote! {
+    #maybe_scope
+    let msg = #deno_core::v8::String::new_from_one_byte(#scope, #message.as_bytes(), #deno_core::v8::NewStringType::Normal).unwrap();
+    let exc = #deno_core::v8::Exception::error(#scope, msg);
+    #scope.throw_exception(exc);
     return;
   })
 }
diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs
index 7f652fe..14d3c3b 100644
--- a/ops/op2/mod.rs
+++ b/ops/op2/mod.rs
@@ -9,14 +9,18 @@ use quote::ToTokens;
 use std::iter::zip;
 use syn2::parse2;
 use syn2::parse_str;
+use syn2::spanned::Spanned;
 use syn2::FnArg;
 use syn2::ItemFn;
+use syn2::Lifetime;
+use syn2::LifetimeParam;
 use syn2::Path;
 use thiserror::Error;
 
 use self::dispatch_fast::generate_dispatch_fast;
 use self::dispatch_slow::generate_dispatch_slow;
 use self::generator_state::GeneratorState;
+use self::signature::is_attribute_special;
 use self::signature::parse_signature;
 use self::signature::Arg;
 use self::signature::SignatureError;
@@ -104,7 +108,12 @@ fn generate_op2(
   // Create a copy of the original function, named "call"
   let call = Ident::new("call", Span::call_site());
   let mut op_fn = func.clone();
-  op_fn.attrs.clear();
+  // Collect non-special attributes
+  let attrs = op_fn
+    .attrs
+    .drain(..)
+    .filter(|attr| !is_attribute_special(attr))
+    .collect::<Vec<_>>();
   op_fn.sig.generics.params.clear();
   op_fn.sig.ident = call.clone();
 
@@ -118,6 +127,15 @@ fn generate_op2(
   }
 
   let signature = parse_signature(func.attrs, func.sig.clone())?;
+  if let Some(ident) = signature.lifetime.as_ref().map(|s| format_ident!("{s}"))
+  {
+    op_fn.sig.generics.params.push(syn2::GenericParam::Lifetime(
+      LifetimeParam::new(Lifetime {
+        apostrophe: op_fn.span(),
+        ident,
+      }),
+    ));
+  }
   let processed_args =
     zip(signature.args.iter(), &func.sig.inputs).collect::<Vec<_>>();
 
@@ -211,6 +229,7 @@ fn generate_op2(
 
   Ok(quote! {
     #[allow(non_camel_case_types)]
+    #(#attrs)*
     #vis struct #name <#(#generic),*> {
       // We need to mark these type parameters as used, so we use a PhantomData
       _unconstructable: ::std::marker::PhantomData<(#(#generic),*)>
@@ -252,6 +271,7 @@ fn generate_op2(
       #slow_fn
 
       #[inline(always)]
+      #(#attrs)*
       #op_fn
     }
   })
@@ -302,7 +322,7 @@ mod tests {
         let tokens =
           generate_op2(config.unwrap(), func).expect("Failed to generate op");
         println!("======== Raw tokens ========:\n{}", tokens.clone());
-        let tree = syn::parse2(tokens).unwrap();
+        let tree = syn2::parse2(tokens).unwrap();
         let actual = prettyplease::unparse(&tree);
         println!("======== Generated ========:\n{}", actual);
         expected_out.push(actual);
diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs
index 5d472fc..dddcc13 100644
--- a/ops/op2/signature.rs
+++ b/ops/op2/signature.rs
@@ -55,6 +55,7 @@ impl ToTokens for NumericArg {
   Copy, Clone, Debug, Eq, PartialEq, IntoStaticStr, EnumString, EnumIter,
 )]
 pub enum V8Arg {
+  Value,
   External,
   Object,
   Array,
@@ -117,6 +118,9 @@ pub enum RefType {
   Mut,
 }
 
+/// Args are not a 1:1 mapping with Rust types, rather they represent broad classes of types that
+/// tend to have similar argument handling characteristics. This may need one more level of indirection
+/// given how many of these types have option variants, however.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum Arg {
   Void,
@@ -127,11 +131,54 @@ pub enum Arg {
   OptionNumeric(NumericArg),
   Slice(RefType, NumericArg),
   Ptr(RefType, NumericArg),
+  OptionV8Local(V8Arg),
   V8Local(V8Arg),
+  V8Global(V8Arg),
+  OptionV8Ref(RefType, V8Arg),
+  V8Ref(RefType, V8Arg),
   Numeric(NumericArg),
   SerdeV8(String),
 }
 
+impl Arg {
+  /// Is this argument virtual? ie: does it come from the æther rather than a concrete JavaScript input
+  /// argument?
+  #[allow(clippy::match_like_matches_macro)]
+  pub fn is_virtual(&self) -> bool {
+    match self {
+      Self::Special(
+        Special::FastApiCallbackOptions
+        | Special::OpState
+        | Special::HandleScope,
+      ) => true,
+      Self::Ref(
+        _,
+        Special::FastApiCallbackOptions
+        | Special::OpState
+        | Special::HandleScope,
+      ) => true,
+      _ => false,
+    }
+  }
+}
+
+enum ParsedType {
+  TSpecial(Special),
+  TV8(V8Arg),
+  // TODO(mmastrac): We need to carry the mut status through somehow
+  TV8Mut(V8Arg),
+  TNumeric(NumericArg),
+}
+
+enum ParsedTypeContainer {
+  CBare(ParsedType),
+  COption(ParsedType),
+  CRcRefCell(ParsedType),
+  COptionV8Local(ParsedType),
+  CV8Local(ParsedType),
+  CV8Global(ParsedType),
+}
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum RetVal {
   Infallible(Arg),
@@ -196,8 +243,14 @@ pub enum ArgError {
   InvalidTypePath(String),
   #[error("Too many attributes")]
   TooManyAttributes,
+  #[error("The type {0} cannot be a reference")]
+  InvalidReference(String),
+  #[error("The type {0} must be a reference")]
+  MissingReference(String),
   #[error("Invalid #[serde] type: {0}")]
   InvalidSerdeType(String),
+  #[error("Invalid #[string] type: {0}")]
+  InvalidStringType(String),
   #[error("Cannot use #[serde] for type: {0}")]
   InvalidSerdeAttributeType(String),
   #[error("Invalid v8 type: {0}")]
@@ -359,6 +412,10 @@ fn parse_attributes(attributes: &[Attribute]) -> Result<Attributes, ArgError> {
   })
 }
 
+pub fn is_attribute_special(attr: &Attribute) -> bool {
+  parse_attribute(attr).is_some()
+}
+
 fn parse_attribute(attr: &Attribute) -> Option<AttributeModifier> {
   let tokens = attr.into_token_stream();
   use syn2 as syn;
@@ -410,12 +467,21 @@ fn parse_return(
   }
 }
 
-fn parse_type_path(attrs: Attributes, tp: &TypePath) -> Result<Arg, ArgError> {
+/// Parse a raw type into a container + type, allowing us to simplify the typechecks elsewhere in
+/// this code.
+fn parse_type_path(
+  attrs: Attributes,
+  is_ref: bool,
+  tp: &TypePath,
+) -> Result<ParsedTypeContainer, ArgError> {
+  use ParsedType::*;
+  use ParsedTypeContainer::*;
+
   if tp.path.segments.len() == 1 {
     let segment = tp.path.segments.first().unwrap().ident.to_string();
     for numeric in NumericArg::iter() {
       if Into::<&'static str>::into(numeric) == segment.as_str() {
-        return Ok(Arg::Numeric(numeric));
+        return Ok(CBare(TNumeric(numeric)));
       }
     }
   }
@@ -423,42 +489,75 @@ fn parse_type_path(attrs: Attributes, tp: &TypePath) -> Result<Arg, ArgError> {
   use syn2 as syn;
 
   let tokens = tp.clone().into_token_stream();
-  std::panic::catch_unwind(|| {
+  let res = std::panic::catch_unwind(|| {
     rules!(tokens => {
       ( $( std :: str  :: )? String ) => {
-        if attrs.primary == Some(AttributeModifier::String) {
-          Ok(Arg::Special(Special::String))
-        } else {
-          Err(ArgError::MissingStringAttribute)
-        }
+        Ok(CBare(TSpecial(Special::String)))
       }
+      // Note that the reference is checked below
       ( $( std :: str  :: )? str ) => {
-        // We should not hit this path with a #[string] argument
-        Err(ArgError::MissingStringAttribute)
+        Ok(CBare(TSpecial(Special::RefStr)))
       }
       ( $( std :: borrow :: )? Cow < str > ) => {
-        if attrs.primary == Some(AttributeModifier::String) {
-          Ok(Arg::Special(Special::CowStr))
-        } else {
-          Err(ArgError::MissingStringAttribute)
-        }
+        Ok(CBare(TSpecial(Special::CowStr)))
       }
-      ( $( std :: ffi :: )? c_void ) => Ok(Arg::Numeric(NumericArg::__VOID__)),
-      ( OpState ) => Ok(Arg::Special(Special::OpState)),
-      ( v8 :: HandleScope ) => Ok(Arg::Special(Special::HandleScope)),
-      ( v8 :: FastApiCallbackOptions ) => Ok(Arg::Special(Special::FastApiCallbackOptions)),
-      ( v8 :: Local < $( $_scope:lifetime , )? v8 :: $v8:ident >) => Ok(Arg::V8Local(parse_v8_type(&v8)?)),
-      ( Rc < RefCell < $ty:ty > > ) => Ok(Arg::RcRefCell(parse_type_special(attrs, &ty)?)),
+      ( $( std :: ffi :: )? c_void ) => Ok(CBare(TNumeric(NumericArg::__VOID__))),
+      ( OpState ) => Ok(CBare(TSpecial(Special::OpState))),
+      ( v8 :: HandleScope $( < $_scope:lifetime >)? ) => Ok(CBare(TSpecial(Special::HandleScope))),
+      ( v8 :: FastApiCallbackOptions ) => Ok(CBare(TSpecial(Special::FastApiCallbackOptions))),
+      ( v8 :: Local < $( $_scope:lifetime , )? v8 :: $v8:ident >) => Ok(CV8Local(TV8(parse_v8_type(&v8)?))),
+      ( v8 :: Global < $( $_scope:lifetime , )? v8 :: $v8:ident >) => Ok(CV8Global(TV8(parse_v8_type(&v8)?))),
+      ( v8 :: $v8:ident ) => Ok(CBare(TV8(parse_v8_type(&v8)?))),
+      ( Rc < RefCell < $ty:ty > > ) => Ok(CRcRefCell(TSpecial(parse_type_special(attrs, &ty)?))),
       ( Option < $ty:ty > ) => {
         match parse_type(attrs, &ty)? {
-          Arg::Special(special) => Ok(Arg::Option(special)),
-          Arg::Numeric(numeric) => Ok(Arg::OptionNumeric(numeric)),
+          Arg::Special(special) => Ok(COption(TSpecial(special))),
+          Arg::Numeric(numeric) => Ok(COption(TNumeric(numeric))),
+          Arg::V8Ref(RefType::Ref, v8) => Ok(COption(TV8(v8))),
+          Arg::V8Ref(RefType::Mut, v8) => Ok(COption(TV8Mut(v8))),
+          Arg::V8Local(v8) => Ok(COptionV8Local(TV8(v8))),
           _ => Err(ArgError::InvalidType(stringify_token(ty)))
         }
       }
       ( $any:ty ) => Err(ArgError::InvalidTypePath(stringify_token(any))),
     })
-  }).map_err(|e| ArgError::InternalError(format!("parse_type_path {e:?}")))?
+  }).map_err(|e| ArgError::InternalError(format!("parse_type_path {e:?}")))??;
+
+  // Ensure that we have the correct reference state. This is a bit awkward but it's
+  // the easiest way to work with the 'rules!' macro above.
+  match res {
+    CBare(
+      TSpecial(Special::RefStr | Special::OpState | Special::HandleScope)
+      | TV8(_),
+    ) => {
+      if !is_ref {
+        return Err(ArgError::MissingReference(stringify_token(tp)));
+      }
+    }
+    _ => {
+      if is_ref {
+        return Err(ArgError::InvalidReference(stringify_token(tp)));
+      }
+    }
+  }
+
+  match res {
+    CBare(TSpecial(Special::RefStr | Special::CowStr | Special::String)) => {
+      if attrs.primary != Some(AttributeModifier::String) {
+        return Err(ArgError::MissingStringAttribute);
+      }
+    }
+    CBare(_) => {
+      if attrs.primary == Some(AttributeModifier::String) {
+        return Err(ArgError::InvalidStringType(stringify_token(tp)));
+      }
+    }
+    _ => {
+      // Ignore for other containers
+    }
+  }
+
+  Ok(res)
 }
 
 fn parse_v8_type(v8: &Ident) -> Result<V8Arg, ArgError> {
@@ -477,12 +576,15 @@ fn parse_type_special(
 }
 
 fn parse_type(attrs: Attributes, ty: &Type) -> Result<Arg, ArgError> {
+  use ParsedType::*;
+  use ParsedTypeContainer::*;
+
   if let Some(primary) = attrs.primary {
     match primary {
       AttributeModifier::Serde => match ty {
         Type::Path(of) => {
           // If this type will parse without #[serde], it is illegal to use this type with #[serde]
-          if parse_type_path(Attributes::default(), of).is_ok() {
+          if parse_type_path(Attributes::default(), false, of).is_ok() {
             return Err(ArgError::InvalidSerdeAttributeType(stringify_token(
               ty,
             )));
@@ -491,25 +593,9 @@ fn parse_type(attrs: Attributes, ty: &Type) -> Result<Arg, ArgError> {
         }
         _ => return Err(ArgError::InvalidSerdeType(stringify_token(ty))),
       },
-      AttributeModifier::String => match ty {
-        Type::Path(of) => {
-          return parse_type_path(attrs, of);
-        }
-        Type::Reference(of) => {
-          let mut_type = if of.mutability.is_some() {
-            RefType::Mut
-          } else {
-            RefType::Ref
-          };
-          let tokens = of.elem.clone().into_token_stream();
-          use syn2 as syn;
-          return rules!(tokens => {
-            (str) => Ok(Arg::Special(Special::RefStr)),
-            ($_ty:ty) => Ok(Arg::Ref(mut_type, parse_type_special(attrs, &of.elem)?)),
-          });
-        }
-        _ => return Err(ArgError::InvalidSerdeType(stringify_token(ty))),
-      },
+      AttributeModifier::String => {
+        // We handle this as part of the normal parsing process
+      }
       AttributeModifier::Smi => {
         return Ok(Arg::Numeric(NumericArg::__SMI__));
       }
@@ -534,8 +620,13 @@ fn parse_type(attrs: Attributes, ty: &Type) -> Result<Arg, ArgError> {
           Arg::Numeric(numeric) => Ok(Arg::Slice(mut_type, numeric)),
           _ => Err(ArgError::InvalidType(stringify_token(ty))),
         },
-        Type::Path(of) => match parse_type_path(attrs, of)? {
-          Arg::Special(special) => Ok(Arg::Ref(mut_type, special)),
+        Type::Path(of) => match parse_type_path(attrs, true, of)? {
+          CBare(TSpecial(Special::RefStr)) => Ok(Arg::Special(Special::RefStr)),
+          COption(TSpecial(Special::RefStr)) => {
+            Ok(Arg::Option(Special::RefStr))
+          }
+          CBare(TV8(v8)) => Ok(Arg::V8Ref(mut_type, v8)),
+          CBare(TSpecial(special)) => Ok(Arg::Ref(mut_type, special)),
           _ => Err(ArgError::InvalidType(stringify_token(ty))),
         },
         _ => Err(ArgError::InvalidType(stringify_token(ty))),
@@ -548,14 +639,26 @@ fn parse_type(attrs: Attributes, ty: &Type) -> Result<Arg, ArgError> {
         RefType::Ref
       };
       match &*of.elem {
-        Type::Path(of) => match parse_type_path(attrs, of)? {
-          Arg::Numeric(numeric) => Ok(Arg::Ptr(mut_type, numeric)),
+        Type::Path(of) => match parse_type_path(attrs, false, of)? {
+          CBare(TNumeric(numeric)) => Ok(Arg::Ptr(mut_type, numeric)),
           _ => Err(ArgError::InvalidType(stringify_token(ty))),
         },
         _ => Err(ArgError::InvalidType(stringify_token(ty))),
       }
     }
-    Type::Path(of) => parse_type_path(attrs, of),
+    Type::Path(of) => match parse_type_path(attrs, false, of)? {
+      CBare(TNumeric(numeric)) => Ok(Arg::Numeric(numeric)),
+      CBare(TSpecial(special)) => Ok(Arg::Special(special)),
+      COption(TNumeric(special)) => Ok(Arg::OptionNumeric(special)),
+      COption(TSpecial(special)) => Ok(Arg::Option(special)),
+      CRcRefCell(TSpecial(special)) => Ok(Arg::RcRefCell(special)),
+      COptionV8Local(TV8(v8)) => Ok(Arg::OptionV8Local(v8)),
+      COption(TV8(v8)) => Ok(Arg::OptionV8Ref(RefType::Ref, v8)),
+      COption(TV8Mut(v8)) => Ok(Arg::OptionV8Ref(RefType::Mut, v8)),
+      CV8Local(TV8(v8)) => Ok(Arg::V8Local(v8)),
+      CV8Global(TV8(v8)) => Ok(Arg::V8Global(v8)),
+      _ => Err(ArgError::InvalidType(stringify_token(ty))),
+    },
     _ => Err(ArgError::InvalidType(stringify_token(ty))),
   }
 }
@@ -635,8 +738,10 @@ mod tests {
       );
     }
     assert_eq!(
-      args_expected,
-      format!("{:?}", sig.args).trim_matches(|c| c == '[' || c == ']')
+      args_expected.replace('\n', " "),
+      format!("{:?}", sig.args)
+        .trim_matches(|c| c == '[' || c == ']')
+        .replace('\n', " ")
     );
     assert_eq!(return_expected, format!("{:?}", sig.ret_val));
   }
@@ -692,6 +797,14 @@ mod tests {
     fn op_print(#[string] msg: &str, is_err: bool) -> Result<(), Error>;
     (Special(RefStr), Numeric(bool)) -> Result(Void)
   );
+  test!(
+    #[string] fn op_lots_of_strings(#[string] s: String, #[string] s2: Option<String>, #[string] s3: Cow<str>) -> String;
+    (Special(String), Option(String), Special(CowStr)) -> Infallible(Special(String))
+  );
+  test!(
+    #[string] fn op_lots_of_option_strings(#[string] s: Option<String>, #[string] s2: Option<&str>, #[string] s3: Option<Cow<str>>) -> Option<String>;
+    (Option(String), Option(RefStr), Option(CowStr)) -> Infallible(Option(String))
+  );
   test!(
     fn op_scope<'s>(#[string] msg: &'s str);
     <'s> (Special(RefStr)) -> Infallible(Void)
@@ -700,6 +813,34 @@ 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> (Special(RefStr)) -> Infallible(Void)
   );
+  test!(
+    fn op_v8_types(s: &mut v8::String, sopt: Option<&mut v8::String>, s2: v8::Local<v8::String>, s3: v8::Global<v8::String>);
+    (V8Ref(Mut, String), OptionV8Ref(Mut, String), V8Local(String), V8Global(String)) -> Infallible(Void)
+  );
+  test!(
+    fn op_v8_scope<'s>(scope: &mut v8::HandleScope<'s>);
+    <'s> (Ref(Mut, HandleScope)) -> Infallible(Void)
+  );
+
+  // Args
+
+  expect_fail!(
+    op_with_bad_string1,
+    ArgError("s", MissingStringAttribute),
+    fn f(s: &str) {}
+  );
+  expect_fail!(
+    op_with_bad_string2,
+    ArgError("s", MissingStringAttribute),
+    fn f(s: String) {}
+  );
+  expect_fail!(
+    op_with_bad_string3,
+    ArgError("s", MissingStringAttribute),
+    fn f(s: Cow<str>) {}
+  );
+
+  // Generics
 
   expect_fail!(op_with_two_lifetimes, TooManyLifetimes, fn f<'a, 'b>() {});
   expect_fail!(


Test patch:
diff --git a/ops/op2/test_cases/sync/clippy_allow.out b/ops/op2/test_cases/sync/clippy_allow.out
new file mode 100644
index 0000000..d032449
--- /dev/null
+++ b/ops/op2/test_cases/sync/clippy_allow.out
@@ -0,0 +1,63 @@
+#[allow(non_camel_case_types)]
+/// This is a doc comment.
+#[allow(clippy::some_annotation)]
+pub struct op_extra_annotation {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_extra_annotation {
+    const NAME: &'static str = stringify!(op_extra_annotation);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_extra_annotation),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new(
+                &[Type::V8Value],
+                CType::Void,
+                Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+            )
+        }),
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 0usize as u8,
+    };
+}
+impl op_extra_annotation {
+    pub const fn name() -> &'static str {
+        stringify!(op_extra_annotation)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_extra_annotation),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: Some({
+                use deno_core::v8::fast_api::Type;
+                use deno_core::v8::fast_api::CType;
+                deno_core::v8::fast_api::FastFunction::new(
+                    &[Type::V8Value],
+                    CType::Void,
+                    Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+                )
+            }),
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 0usize as u8,
+        }
+    }
+    fn v8_fn_ptr_fast(_: deno_core::v8::Local<deno_core::v8::Object>) -> () {
+        let result = Self::call();
+        result
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let result = Self::call();
+    }
+    #[inline(always)]
+    /// This is a doc comment.
+    #[allow(clippy::some_annotation)]
+    pub fn call() -> () {}
+}
diff --git a/ops/op2/test_cases/sync/clippy_allow.rs b/ops/op2/test_cases/sync/clippy_allow.rs
new file mode 100644
index 0000000..009f279
--- /dev/null
+++ b/ops/op2/test_cases/sync/clippy_allow.rs
@@ -0,0 +1,6 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+/// This is a doc comment.
+#[op2(fast)]
+#[allow(clippy::some_annotation)]
+pub fn op_extra_annotation() -> () {}
diff --git a/ops/op2/test_cases/sync/doc_comment.out b/ops/op2/test_cases/sync/doc_comment.out
index d7e8005..c98789f 100644
--- a/ops/op2/test_cases/sync/doc_comment.out
+++ b/ops/op2/test_cases/sync/doc_comment.out
@@ -1,4 +1,5 @@
 #[allow(non_camel_case_types)]
+/// This is a doc comment.
 pub struct op_has_doc_comment {
     _unconstructable: ::std::marker::PhantomData<()>,
 }
@@ -55,5 +56,6 @@ impl op_has_doc_comment {
         let result = Self::call();
     }
     #[inline(always)]
+    /// This is a doc comment.
     pub fn call() -> () {}
 }
diff --git a/ops/op2/test_cases/sync/v8_handlescope.out b/ops/op2/test_cases/sync/v8_handlescope.out
new file mode 100644
index 0000000..45f73a7
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_handlescope.out
@@ -0,0 +1,65 @@
+#[allow(non_camel_case_types)]
+struct op_handlescope {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_handlescope {
+    const NAME: &'static str = stringify!(op_handlescope);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_handlescope),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: None,
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 2usize as u8,
+    };
+}
+impl op_handlescope {
+    pub const fn name() -> &'static str {
+        stringify!(op_handlescope)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_handlescope),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 2usize as u8,
+        }
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let scope = &mut 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 arg1 = args.get(0usize as i32);
+        let Ok(mut arg1) = deno_core::v8::Local::<
+            deno_core::v8::String,
+        >::try_from(arg1) else {
+        let msg = deno_core::v8::String::new_from_one_byte(
+                scope,
+                "expected v8::String".as_bytes(),
+                deno_core::v8::NewStringType::Normal,
+            )
+            .unwrap();
+        let exc = deno_core::v8::Exception::error(scope, msg);
+        scope.throw_exception(exc);
+        return;
+    };
+        let arg0 = scope;
+        let result = Self::call(arg0, arg1);
+        rv.set(result.into())
+    }
+    #[inline(always)]
+    fn call(
+        scope: &v8::HandleScope,
+        str2: v8::Local<v8::String>,
+    ) -> v8::Local<v8::String> {}
+}
diff --git a/ops/op2/test_cases/sync/v8_handlescope.rs b/ops/op2/test_cases/sync/v8_handlescope.rs
new file mode 100644
index 0000000..25657e5
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_handlescope.rs
@@ -0,0 +1,5 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+fn op_handlescope(scope: &v8::HandleScope, str2: v8::Local<v8::String>) -> v8::Local<v8::String> {
+}
diff --git a/ops/op2/test_cases/sync/v8_lifetime.out b/ops/op2/test_cases/sync/v8_lifetime.out
new file mode 100644
index 0000000..b922944
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_lifetime.out
@@ -0,0 +1,61 @@
+#[allow(non_camel_case_types)]
+pub struct op_v8_lifetime {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_v8_lifetime {
+    const NAME: &'static str = stringify!(op_v8_lifetime);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_v8_lifetime),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: None,
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 1usize as u8,
+    };
+}
+impl op_v8_lifetime {
+    pub const fn name() -> &'static str {
+        stringify!(op_v8_lifetime)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_v8_lifetime),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 1usize as u8,
+        }
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let scope = &mut 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 = args.get(0usize as i32);
+        let Ok(mut arg0) = deno_core::v8::Local::<
+            deno_core::v8::String,
+        >::try_from(arg0) else {
+        let msg = deno_core::v8::String::new_from_one_byte(
+                scope,
+                "expected v8::String".as_bytes(),
+                deno_core::v8::NewStringType::Normal,
+            )
+            .unwrap();
+        let exc = deno_core::v8::Exception::error(scope, msg);
+        scope.throw_exception(exc);
+        return;
+    };
+        let result = Self::call(arg0);
+        rv.set(result.into())
+    }
+    #[inline(always)]
+    pub fn call<'s>(s: v8::Local<'s, v8::String>) -> v8::Local<'s, v8::String> {}
+}
diff --git a/ops/op2/test_cases/sync/v8_lifetime.rs b/ops/op2/test_cases/sync/v8_lifetime.rs
new file mode 100644
index 0000000..3050a20
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_lifetime.rs
@@ -0,0 +1,4 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+pub fn op_v8_lifetime<'s>(s: v8::Local<'s, v8::String>) -> v8::Local<'s, v8::String> {}
diff --git a/ops/op2/test_cases/sync/v8_ref_option.out b/ops/op2/test_cases/sync/v8_ref_option.out
new file mode 100644
index 0000000..384f7c0
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_ref_option.out
@@ -0,0 +1,83 @@
+#[allow(non_camel_case_types)]
+pub struct op_v8_lifetime {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_v8_lifetime {
+    const NAME: &'static str = stringify!(op_v8_lifetime);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_v8_lifetime),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: None,
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 2usize as u8,
+    };
+}
+impl op_v8_lifetime {
+    pub const fn name() -> &'static str {
+        stringify!(op_v8_lifetime)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_v8_lifetime),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 2usize as u8,
+        }
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let scope = &mut unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+            &*info
+        });
+        let arg0 = args.get(0usize as i32);
+        let arg0 = if arg0.is_null_or_undefined() {
+            None
+        } else {
+            let Ok(mut arg0) = deno_core::v8::Local::<
+                deno_core::v8::String,
+            >::try_from(arg0) else {
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    scope,
+                    "expected v8::String".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            Some(arg0)
+        };
+        let arg0 = arg0.as_ref().map(|v| ::std::ops::Deref::deref(v));
+        let arg1 = args.get(1usize as i32);
+        let arg1 = if arg1.is_null_or_undefined() {
+            None
+        } else {
+            let Ok(mut arg1) = deno_core::v8::Local::<
+                deno_core::v8::String,
+            >::try_from(arg1) else {
+            let msg = deno_core::v8::String::new_from_one_byte(
+                    scope,
+                    "expected v8::String".as_bytes(),
+                    deno_core::v8::NewStringType::Normal,
+                )
+                .unwrap();
+            let exc = deno_core::v8::Exception::error(scope, msg);
+            scope.throw_exception(exc);
+            return;
+        };
+            Some(arg1)
+        };
+        let arg1 = arg1.as_mut().map(|v| ::std::ops::DerefMut::deref_mut(v));
+        let result = Self::call(arg0, arg1);
+    }
+    #[inline(always)]
+    pub fn call<'s>(s: Option<&v8::String>, s2: Option<&mut v8::String>) {}
+}
diff --git a/ops/op2/test_cases/sync/v8_ref_option.rs b/ops/op2/test_cases/sync/v8_ref_option.rs
new file mode 100644
index 0000000..3715b86
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_ref_option.rs
@@ -0,0 +1,5 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+pub fn op_v8_lifetime<'s>(s: Option<&v8::String>, s2: Option<&mut v8::String>) {}
+
diff --git a/ops/op2/test_cases/sync/v8_string.out b/ops/op2/test_cases/sync/v8_string.out
new file mode 100644
index 0000000..56d4443
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_string.out
@@ -0,0 +1,76 @@
+#[allow(non_camel_case_types)]
+struct op_v8_string {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_v8_string {
+    const NAME: &'static str = stringify!(op_v8_string);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl {
+        name: stringify!(op_v8_string),
+        v8_fn_ptr: Self::v8_fn_ptr as _,
+        enabled: true,
+        fast_fn: None,
+        is_async: false,
+        is_unstable: false,
+        is_v8: false,
+        arg_count: 2usize as u8,
+    };
+}
+impl op_v8_string {
+    pub const fn name() -> &'static str {
+        stringify!(op_v8_string)
+    }
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        deno_core::_ops::OpDecl {
+            name: stringify!(op_v8_string),
+            v8_fn_ptr: Self::v8_fn_ptr as _,
+            enabled: true,
+            fast_fn: None,
+            is_async: false,
+            is_unstable: false,
+            is_v8: false,
+            arg_count: 2usize as u8,
+        }
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let scope = &mut 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 = args.get(0usize as i32);
+        let Ok(mut arg0) = deno_core::v8::Local::<
+            deno_core::v8::String,
+        >::try_from(arg0) else {
+        let msg = deno_core::v8::String::new_from_one_byte(
+                scope,
+                "expected v8::String".as_bytes(),
+                deno_core::v8::NewStringType::Normal,
+            )
+            .unwrap();
+        let exc = deno_core::v8::Exception::error(scope, msg);
+        scope.throw_exception(exc);
+        return;
+    };
+        let arg0 = &arg0;
+        let arg1 = args.get(1usize as i32);
+        let Ok(mut arg1) = deno_core::v8::Local::<
+            deno_core::v8::String,
+        >::try_from(arg1) else {
+        let msg = deno_core::v8::String::new_from_one_byte(
+                scope,
+                "expected v8::String".as_bytes(),
+                deno_core::v8::NewStringType::Normal,
+            )
+            .unwrap();
+        let exc = deno_core::v8::Exception::error(scope, msg);
+        scope.throw_exception(exc);
+        return;
+    };
+        let result = Self::call(arg0, arg1);
+        rv.set(result.into())
+    }
+    #[inline(always)]
+    fn call(str1: &v8::String, str2: v8::Local<v8::String>) -> v8::Local<v8::String> {}
+}
diff --git a/ops/op2/test_cases/sync/v8_string.rs b/ops/op2/test_cases/sync/v8_string.rs
new file mode 100644
index 0000000..db0c350
--- /dev/null
+++ b/ops/op2/test_cases/sync/v8_string.rs
@@ -0,0 +1,5 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+#[op2]
+fn op_v8_string(str1: &v8::String, str2: v8::Local<v8::String>) -> v8::Local<v8::String> {
+}