Skip to content

Commit

Permalink
Add #[required(<count>)
Browse files Browse the repository at this point in the history
  • Loading branch information
littledivy committed Nov 21, 2024
1 parent 06a310e commit d33a034
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 14 deletions.
19 changes: 17 additions & 2 deletions ops/op2/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ pub(crate) struct MacroConfig {
pub reentrant: bool,
/// Marks an op as a method on a wrapped object.
pub method: Option<String>,
/// Same as method name but also used by static and constructor ops.
pub self_name: Option<String>,
/// Marks an op as a constructor
pub constructor: bool,
/// Marks an op as a static member
Expand All @@ -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 {
Expand Down Expand Up @@ -126,6 +130,13 @@ impl MacroConfig {
})
})
.map_err(|_| Op2Error::PatternMatchFailed("attribute", flag))?;
} else if flag.starts_with("required(") {
let tokens = syn::parse_str::<TokenTree>(&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));
}
Expand Down Expand Up @@ -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())
)
}
})
})
Expand Down
36 changes: 36 additions & 0 deletions ops/op2/dispatch_slow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand All @@ -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
Expand Down Expand Up @@ -227,6 +235,34 @@ 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"
};
gs_quote!(generator_state(fn_args, scope, self_ty) =>
(if #fn_args.length() < #required as i32 {
let msg = format!(
"Failed to execute '{}' on '{}': {} {} required, but only {} present",
Self::name(),
stringify!(#self_ty),
#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) =>
Expand Down
2 changes: 1 addition & 1 deletion ops/op2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
15 changes: 11 additions & 4 deletions ops/op2/object_wrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -82,8 +84,10 @@ 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(|attr| is_attribute_special(attr));

/* Convert snake_case to camelCase */
method.sig.ident = format_ident!(
Expand All @@ -93,15 +97,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);
Expand All @@ -120,6 +125,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);
}
Expand Down
32 changes: 25 additions & 7 deletions ops/op2/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -771,6 +773,7 @@ impl AttributeModifier {
AttributeModifier::State => "state",
AttributeModifier::Global => "global",
AttributeModifier::CppGcResource => "cppgc",
AttributeModifier::Ignore => "ignore",
}
}
}
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -1181,24 +1182,30 @@ 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)
}
}

if attrs.is_empty() {
return Ok(Attributes::default());
}
if attrs.len() > 1 {
return Err(AttributeError::TooManyAttributes);
}
Ok(Attributes {
primary: Some(*attrs.first().unwrap()),
})
}

/// 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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const { DOMPoint } = Deno.core.ops;

const point = new DOMPoint(1, 2, 3, 4);

point.x;

point.stuff();

7 changes: 7 additions & 0 deletions testing/checkin/runner/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ impl GarbageCollected for DOMPoint {}

#[op2]
impl DOMPoint {
#[fast]
#[required(1)]
fn stuff(&self) {
println!("stuff");
}

#[constructor]
#[cppgc]
fn new(
Expand All @@ -97,6 +103,7 @@ impl DOMPoint {
}
}

#[required(1)]
#[static_method]
#[cppgc]
fn from_point(
Expand Down

0 comments on commit d33a034

Please sign in to comment.