diff --git a/ops/op2/config.rs b/ops/op2/config.rs index 1b822d2d6..150b998da 100644 --- a/ops/op2/config.rs +++ b/ops/op2/config.rs @@ -24,6 +24,8 @@ pub(crate) struct MacroConfig { pub reentrant: bool, /// Marks an op as a method on a wrapped object. pub method: Option, + /// Same as method name but also used by static and constructor ops. + pub self_name: Option, /// Marks an op as a constructor pub constructor: bool, /// Marks an op as a static member @@ -36,6 +38,8 @@ pub(crate) struct MacroConfig { pub setter: bool, /// Marks an op to have it collect stack trace of the call site in the OpState. pub stack_trace: bool, + /// Total required number of arguments for the op. + pub required: u8, } impl MacroConfig { @@ -126,6 +130,13 @@ impl MacroConfig { }) }) .map_err(|_| Op2Error::PatternMatchFailed("attribute", flag))?; + } else if flag.starts_with("required(") { + let tokens = syn::parse_str::(&flag[9..flag.len() - 1])? + .into_token_stream(); + config.required = tokens + .to_string() + .parse() + .map_err(|_| Op2Error::InvalidAttribute(flag))?; } else { return Err(Op2Error::InvalidAttribute(flag)); } @@ -182,8 +193,12 @@ impl MacroConfig { ( $($flags:tt $( ( $( $args:ty ),* ) )? ),+ ) => { Self::from_token_trees(flags, args) } - ( # [ $($flags:tt),+ ] ) => { - Self::from_flags(flags.into_iter().map(|flag| flag.to_string())) + ( $( #[$attr:meta] )* ) => { + Self::from_flags( + attr. + into_iter(). + map(|attr| attr.to_token_stream().to_string()) + ) } }) }) diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs index cf9973cab..967ceb392 100644 --- a/ops/op2/dispatch_slow.rs +++ b/ops/op2/dispatch_slow.rs @@ -120,6 +120,13 @@ pub(crate) fn generate_dispatch_slow( quote!() }; + let with_required_check = if generator_state.needs_args && config.required > 0 + { + with_required_check(generator_state, config.required) + } else { + quote!() + }; + let with_self = if generator_state.needs_self { with_self(generator_state, &signature.ret_val) .map_err(V8SignatureMappingError::NoSelfMapping)? @@ -143,6 +150,7 @@ pub(crate) fn generate_dispatch_slow( #with_scope #with_retval #with_args + #with_required_check #with_opctx #with_self #with_isolate @@ -227,6 +235,48 @@ pub(crate) fn with_fn_args( ) } +pub(crate) fn with_required_check( + generator_state: &mut GeneratorState, + required: u8, +) -> TokenStream { + generator_state.needs_scope = true; + let arguments_lit = if required > 1 { + "arguments" + } else { + "argument" + }; + + let prefix = if generator_state.needs_self { + format!( + "Failed to execute '{}' on '{}': ", + generator_state.name, generator_state.self_ty + ) + } else if generator_state.use_this_cppgc { + format!("Failed to construct '{}': ", generator_state.self_ty) + } else { + format!( + "Failed to execute '{}.{}': ", + generator_state.self_ty, generator_state.name + ) + }; + + gs_quote!(generator_state(fn_args, scope) => + (if #fn_args.length() < #required as i32 { + let msg = format!( + "{}{} {} required, but only {} present", + #prefix, + #required, + #arguments_lit, + #fn_args.length() + ); + let msg = deno_core::v8::String::new(&mut #scope, &msg).unwrap(); + let exception = deno_core::v8::Exception::type_error(&mut #scope, msg.into()); + #scope.throw_exception(exception); + return 1; + }) + ) +} + pub(crate) fn with_opctx(generator_state: &mut GeneratorState) -> TokenStream { generator_state.needs_args = true; gs_quote!(generator_state(opctx, fn_args) => diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 0826442a6..a0855d12b 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -160,7 +160,7 @@ pub(crate) fn generate_op2( Ident::new("v8_fn_ptr_fast_metrics", Span::call_site()); let fast_api_callback_options = Ident::new("fast_api_callback_options", Span::call_site()); - let self_ty = if let Some(ref ty) = config.method { + let self_ty = if let Some(ref ty) = config.self_name { format_ident!("{ty}") } else { Ident::new("UNINIT", Span::call_site()) diff --git a/ops/op2/object_wrap.rs b/ops/op2/object_wrap.rs index fc16f4929..a9e0a4cbc 100644 --- a/ops/op2/object_wrap.rs +++ b/ops/op2/object_wrap.rs @@ -12,6 +12,8 @@ use crate::op2::generate_op2; use crate::op2::MacroConfig; use crate::op2::Op2Error; +use super::signature::is_attribute_special; + // Object wrap for Cppgc-backed objects // // This module generates the glue code declarations @@ -82,8 +84,8 @@ pub(crate) fn generate_impl_ops( for item in item.items { if let ImplItem::Fn(mut method) = item { - /* First attribute idents, for all functions in block */ - let attrs = method.attrs.swap_remove(0); + let (item_fn_attrs, attrs) = + method.attrs.into_iter().partition(is_attribute_special); /* Convert snake_case to camelCase */ method.sig.ident = format_ident!( @@ -93,15 +95,16 @@ pub(crate) fn generate_impl_ops( let ident = method.sig.ident.clone(); let func = ItemFn { - attrs: method.attrs, + attrs: item_fn_attrs, vis: method.vis, sig: method.sig, block: Box::new(method.block), }; let mut config = MacroConfig::from_tokens(quote! { - #attrs + #(#attrs)* })?; + if config.constructor { if constructor.is_some() { return Err(Op2Error::MultipleConstructors); @@ -120,6 +123,8 @@ pub(crate) fn generate_impl_ops( config.method = Some(self_ty_ident.clone()); } + config.self_name = Some(self_ty_ident.clone()); + let op = generate_op2(config, func)?; tokens.extend(op); } diff --git a/ops/op2/signature.rs b/ops/op2/signature.rs index 3d035e2d0..9352f32c6 100644 --- a/ops/op2/signature.rs +++ b/ops/op2/signature.rs @@ -755,6 +755,8 @@ pub enum AttributeModifier { Number, /// #[cppgc], for a resource backed managed by cppgc. CppGcResource, + /// Any attribute that we may want to omit if not syntactically valid. + Ignore, } impl AttributeModifier { @@ -771,6 +773,7 @@ impl AttributeModifier { AttributeModifier::State => "state", AttributeModifier::Global => "global", AttributeModifier::CppGcResource => "cppgc", + AttributeModifier::Ignore => "ignore", } } } @@ -809,8 +812,6 @@ pub enum AttributeError { InvalidAttribute(String), #[error("Invalid inner attribute (#![attr]) in this position. Use an equivalent outer attribute (#[attr]) on the function instead.")] InvalidInnerAttribute, - #[error("Too many attributes")] - TooManyAttributes, } #[derive(Error, Debug)] @@ -1181,6 +1182,9 @@ fn parse_attributes( let mut attrs = vec![]; for attr in attributes { if let Some(attr) = parse_attribute(attr)? { + if attr == AttributeModifier::Ignore { + continue; + } attrs.push(attr) } } @@ -1188,9 +1192,6 @@ fn parse_attributes( if attrs.is_empty() { return Ok(Attributes::default()); } - if attrs.len() > 1 { - return Err(AttributeError::TooManyAttributes); - } Ok(Attributes { primary: Some(*attrs.first().unwrap()), }) @@ -1198,7 +1199,13 @@ fn parse_attributes( /// Is this a special attribute that we understand? pub fn is_attribute_special(attr: &Attribute) -> bool { - parse_attribute(attr).unwrap_or_default().is_some() + parse_attribute(attr) + .unwrap_or_default() + .and_then(|attr| match attr { + AttributeModifier::Ignore => None, + _ => Some(()), + }) + .is_some() // this is kind of ugly, but #[meta(..)] is the only // attribute that we want to omit from the generated code // that doesn't have a semantic meaning @@ -1239,10 +1246,18 @@ fn parse_attribute( (#[cppgc]) => Some(AttributeModifier::CppGcResource), (#[to_v8]) => Some(AttributeModifier::ToV8), (#[from_v8]) => Some(AttributeModifier::FromV8), + (#[required ($_attr:literal)]) => Some(AttributeModifier::Ignore), + (#[method ($_attr:literal)]) => Some(AttributeModifier::Ignore), + (#[method]) => Some(AttributeModifier::Ignore), + (#[getter]) => Some(AttributeModifier::Ignore), + (#[setter]) => Some(AttributeModifier::Ignore), + (#[fast]) => Some(AttributeModifier::Ignore), + (#[static_method]) => Some(AttributeModifier::Ignore), + (#[constructor]) => Some(AttributeModifier::Ignore), (#[allow ($_rule:path)]) => None, (#[doc = $_attr:literal]) => None, (#[cfg $_cfg:tt]) => None, - (#[meta ($($_key: ident = $_value: literal),*)]) => None, + (#[meta ($($_key: ident = $_value: literal),*)]) => Some(AttributeModifier::Ignore), }) }).map_err(|_| AttributeError::InvalidAttribute(stringify_token(attr)))?; Ok(res) @@ -1506,6 +1521,9 @@ pub(crate) fn parse_type( if let Some(primary) = attrs.primary { match primary { + AttributeModifier::Ignore => { + unreachable!(); + } AttributeModifier::CppGcResource => return parse_cppgc(position, ty), AttributeModifier::FromV8 if position == Position::RetVal => { return Err(ArgError::InvalidAttributePosition( diff --git a/testing/checkin/runner/ops.rs b/testing/checkin/runner/ops.rs index 8ac41b17c..042f49e3c 100644 --- a/testing/checkin/runner/ops.rs +++ b/testing/checkin/runner/ops.rs @@ -97,6 +97,7 @@ impl DOMPoint { } } + #[required(1)] #[static_method] #[cppgc] fn from_point(