diff --git a/starlark/src/__derive_refs/sig.rs b/starlark/src/__derive_refs/sig.rs index cd056de45..287a14d1c 100644 --- a/starlark/src/__derive_refs/sig.rs +++ b/starlark/src/__derive_refs/sig.rs @@ -52,3 +52,8 @@ pub fn parameter_spec( kwargs, ) } + +/// `ParametersSpec` for a function which accepts `&Arguments`. +pub fn parameter_spec_for_arguments(name: &'static str) { + parameter_spec(name, &[], &[], true, &[], true); +} diff --git a/starlark_derive/src/module/param_spec.rs b/starlark_derive/src/module/param_spec.rs index 0603cd6a6..591d64ea0 100644 --- a/starlark_derive/src/module/param_spec.rs +++ b/starlark_derive/src/module/param_spec.rs @@ -120,12 +120,6 @@ impl<'a> ParamSpec<'a> { named_only.push(arg); last_param_style = CurrentParamStyle::NamedOnly; } - StarArgPassStyle::Arguments => { - return Err(syn::Error::new( - arg.span, - "unreachable: signature is not meant to be created for `&Arguments`", - )); - } } let optional = arg.default.is_some() || arg.is_option(); diff --git a/starlark_derive/src/module/parse/fun.rs b/starlark_derive/src/module/parse/fun.rs index f90c09215..d3bdc10b3 100644 --- a/starlark_derive/src/module/parse/fun.rs +++ b/starlark_derive/src/module/parse/fun.rs @@ -39,10 +39,12 @@ use crate::module::parse::is_mut_something; use crate::module::parse::is_ref_something; use crate::module::parse::parse_visibility; use crate::module::parse::ModuleKind; +use crate::module::typ::RegularParams; use crate::module::typ::SpecialParam; use crate::module::typ::StarArg; use crate::module::typ::StarArgPassStyle; use crate::module::typ::StarArgSource; +use crate::module::typ::StarArguments; use crate::module::typ::StarAttr; use crate::module::typ::StarFun; use crate::module::typ::StarFunSource; @@ -302,7 +304,7 @@ pub(crate) fn parse_fun(func: ItemFn, module_kind: ModuleKind) -> syn::Result = None; for (i, arg) in func.sig.inputs.into_iter().enumerate() { let span = arg.span(); let parsed_arg = parse_arg(arg, has_v, seen_star_args, module_kind, i)?; @@ -323,8 +325,29 @@ pub(crate) fn parse_fun(func: ItemFn, module_kind: ModuleKind) -> syn::Result { + return Err(syn::Error::new( + span, + "Cannot mix `&Arguments` and regular params", + )); + } + RegularParams::Unpack(args) => args.push(arg), + } } + StarArgOrSpecial::Arguments(arguments) => match args { + None => args = Some(RegularParams::Arguments(arguments)), + Some(RegularParams::Unpack(_)) => { + return Err(syn::Error::new( + span, + "Cannot mix `&Arguments` and regular params", + )); + } + Some(RegularParams::Arguments(_)) => { + return Err(syn::Error::new(span, "Duplicate `&Arguments` parameter")); + } + }, StarArgOrSpecial::This(this_param) => { let expecting_this = module_kind == ModuleKind::Methods && i == 0; if !expecting_this { @@ -356,7 +379,7 @@ pub(crate) fn parse_fun(func: ItemFn, module_kind: ModuleKind) -> syn::Result syn::Result StarFunSource::Arguments, + RegularParams::Unpack(args) => resolve_args(args)?, + }; let fun = StarFun { name: func.sig.ident, @@ -424,43 +451,38 @@ pub(crate) fn parse_fun(func: ItemFn, module_kind: ModuleKind) -> syn::Result syn::Result { - if args.len() == 1 && args[0].pass_style == StarArgPassStyle::Arguments { - args[0].source = StarArgSource::Parameters; - Ok(StarFunSource::Arguments) + let use_arguments = args.iter().any(|x| x.requires_signature()); + if use_arguments { + let mut count = 0; + for x in args.iter_mut() { + x.source = StarArgSource::Argument(count); + count += 1; + } + Ok(StarFunSource::Signature { count }) } else { - let use_arguments = args.iter().any(|x| x.requires_signature()); - if use_arguments { - let mut count = 0; - for x in args.iter_mut() { - x.source = StarArgSource::Argument(count); - count += 1; - } - Ok(StarFunSource::Signature { count }) - } else { - let mut required = 0; - let mut optional = 0; - let mut kwargs = false; - for x in args.iter_mut() { - if x.pass_style == StarArgPassStyle::Kwargs { - if kwargs { - return Err(syn::Error::new(x.span, "Duplicate `**kwargs` parameter")); - } - x.source = StarArgSource::Kwargs; - kwargs = true; - } else if optional == 0 && x.default.is_none() && !x.is_option() { - x.source = StarArgSource::Required(required); - required += 1; - } else { - x.source = StarArgSource::Optional(optional); - optional += 1; + let mut required = 0; + let mut optional = 0; + let mut kwargs = false; + for x in args.iter_mut() { + if x.pass_style == StarArgPassStyle::Kwargs { + if kwargs { + return Err(syn::Error::new(x.span, "Duplicate `**kwargs` parameter")); } + x.source = StarArgSource::Kwargs; + kwargs = true; + } else if optional == 0 && x.default.is_none() && !x.is_option() { + x.source = StarArgSource::Required(required); + required += 1; + } else { + x.source = StarArgSource::Optional(optional); + optional += 1; } - Ok(StarFunSource::Positional { - required, - optional, - kwargs, - }) } + Ok(StarFunSource::Positional { + required, + optional, + kwargs, + }) } } @@ -552,6 +574,8 @@ enum StarArgOrSpecial { This(ThisParam), /// Function parameters. StarArg(StarArg), + /// `&Arguments`. + Arguments(StarArguments), /// `&mut Evaluator`. Eval(SpecialParam), /// `&Heap`. @@ -635,6 +659,36 @@ fn parse_this_param( }) } +fn parse_arguments_param( + span: Span, + mutability: Option, + ident: &Ident, + ty: &syn::Type, + attrs: FnParamAttrs, +) -> syn::Result { + let FnParamAttrs { + default, + this, + pos_only, + named_only, + args, + kwargs, + unused_attrs, + } = attrs; + if default.is_some() || this || pos_only || named_only || args || kwargs { + return Err(syn::Error::new( + span, + "Attributes are not compatible with `&Arguments` parameter", + )); + } + Ok(StarArguments { + other_attrs: unused_attrs, + mutability, + ident: ident.clone(), + ty: ty.clone(), + }) +} + #[allow(clippy::collapsible_else_if)] fn parse_arg( x: FnArg, @@ -711,45 +765,47 @@ fn parse_arg( let arguments = is_ref_something(&ty, "Arguments"); + if arguments { + return Ok(StarArgOrSpecial::Arguments(parse_arguments_param( + span, + ident.mutability, + &ident.ident, + &ty, + param_attrs, + )?)); + } + let pass_style = match ( param_attrs.args, param_attrs.kwargs, seen_star_args, param_attrs.pos_only, param_attrs.named_only, - arguments, ) { - (true, _, _, _, _, false) => StarArgPassStyle::Args, - (_, true, _, _, _, false) => StarArgPassStyle::Kwargs, - (_, _, true, true, _, false) => { + (true, _, _, _, _) => StarArgPassStyle::Args, + (_, true, _, _, _) => StarArgPassStyle::Kwargs, + (_, _, true, true, _) => { return Err(syn::Error::new( span, "Positional-only arguments cannot follow *args", )); } - (false, false, true, false, _, false) => StarArgPassStyle::NamedOnly, + (false, false, true, false, _) => StarArgPassStyle::NamedOnly, // TODO(nga): currently, without `#[starlark(require = named)]` // and without `#[starlark(require = pos)]`, parameter is positional-or-named. // We want to change that: either make it positional by default, // or require explicit `#[starlark(pos, named)]`. // Discussion there: // https://fb.workplace.com/groups/1267349253953900/posts/1299495914072567 - (false, false, false, false, false, false) => StarArgPassStyle::Default, - (false, false, false, true, false, false) => StarArgPassStyle::PosOnly, - (false, false, false, false, true, false) => StarArgPassStyle::NamedOnly, - (false, false, false, true, true, false) => { + (false, false, false, false, false) => StarArgPassStyle::Default, + (false, false, false, true, false) => StarArgPassStyle::PosOnly, + (false, false, false, false, true) => StarArgPassStyle::NamedOnly, + (false, false, false, true, true) => { return Err(syn::Error::new( span, "Function parameter cannot be both positional-only and named-only", )); } - (false, false, _, false, false, true) => StarArgPassStyle::Arguments, - (_, _, _, _, _, true) => { - return Err(syn::Error::new( - span, - "`&Arguments` parameter type is incompatible with annotations", - )); - } }; Ok(StarArgOrSpecial::StarArg(StarArg { span, diff --git a/starlark_derive/src/module/render/fun.rs b/starlark_derive/src/module/render/fun.rs index 03934c391..535c920b0 100644 --- a/starlark_derive/src/module/render/fun.rs +++ b/starlark_derive/src/module/render/fun.rs @@ -28,6 +28,7 @@ use syn::Lit; use crate::module::param_spec::ParamSpec; use crate::module::render::render_starlark_return_type; use crate::module::render::render_starlark_type; +use crate::module::typ::RegularParams; use crate::module::typ::SpecialParam; use crate::module::typ::StarArg; use crate::module::typ::StarArgSource; @@ -138,7 +139,7 @@ impl StarFun { /// Non-special params. fn binding_params_arg(&self) -> syn::Result<(Vec, TokenStream, Vec)> { let Bindings { prepare, bindings } = render_binding(self)?; - let binding_params: Vec<_> = bindings.iter().map(|b| b.render_param()).collect(); + let binding_params: Vec<_> = bindings.iter().map(|b| b.param.clone()).collect(); let binding_args: Vec<_> = bindings.iter().map(|b| b.render_arg()).collect(); Ok((binding_params, prepare, binding_args)) } @@ -329,25 +330,20 @@ struct Bindings { // Given __args and __signature (if render_signature was Some) // create bindings for all the arguments fn render_binding(x: &StarFun) -> syn::Result { - match x.source { - StarFunSource::Arguments => { - let [arg] = x.args.as_slice() else { - return Err(syn::Error::new( - x.span(), - "&Arguments function can have only &Arguments parsed parameter", - )); - }; - Ok(Bindings { - prepare: TokenStream::new(), - bindings: vec![BindingArg { - arg: (*arg).clone(), - expr: syn::parse_quote! { parameters }, - }], - }) - } - StarFunSource::Signature { count } => { - let bind_args: Vec = x - .args + match (&x.args, &x.source) { + (RegularParams::Arguments(arguments), StarFunSource::Arguments) => Ok(Bindings { + prepare: TokenStream::new(), + bindings: vec![BindingArg { + param: arguments.reconstruct_param(), + expr: syn::parse_quote! { parameters }, + }], + }), + (RegularParams::Arguments(_), _) | (_, StarFunSource::Arguments) => Err(syn::Error::new( + x.span(), + "Inconsistent params/source (internal error)", + )), + (RegularParams::Unpack(args), StarFunSource::Signature { count }) => { + let bind_args: Vec = args .iter() .map(render_binding_arg) .collect::>()?; @@ -360,13 +356,15 @@ fn render_binding(x: &StarFun) -> syn::Result { bindings: bind_args, }) } - StarFunSource::Positional { - required, - optional, - kwargs: false, - } => { - let bind_args = x - .args + ( + RegularParams::Unpack(args), + StarFunSource::Positional { + required, + optional, + kwargs: false, + }, + ) => { + let bind_args = args .iter() .map(render_binding_arg) .collect::>()?; @@ -379,13 +377,15 @@ fn render_binding(x: &StarFun) -> syn::Result { bindings: bind_args, }) } - StarFunSource::Positional { - required, - optional, - kwargs: true, - } => { - let bind_args = x - .args + ( + RegularParams::Unpack(args), + StarFunSource::Positional { + required, + optional, + kwargs: true, + }, + ) => { + let bind_args = args .iter() .map(render_binding_arg) .collect::>()?; @@ -403,19 +403,10 @@ fn render_binding(x: &StarFun) -> syn::Result { struct BindingArg { expr: syn::Expr, - arg: StarArg, + param: syn::PatType, } impl BindingArg { - fn render_param(&self) -> syn::PatType { - pat_type( - &self.arg.attrs, - self.arg.mutable, - &self.arg.name, - &self.arg.ty, - ) - } - fn render_arg(&self) -> syn::Expr { self.expr.clone() } @@ -484,7 +475,7 @@ fn render_binding_arg(arg: &StarArg) -> syn::Result { Ok(BindingArg { expr: next, - arg: arg.clone(), + param: pat_type(&arg.attrs, arg.mutable, &arg.name, &arg.ty), }) } @@ -492,31 +483,41 @@ fn render_binding_arg(arg: &StarArg) -> syn::Result { // Or return None if you don't need a signature fn render_signature(x: &StarFun) -> syn::Result { let name_str = ident_string(&x.name); - let ParametersSpecArgs { - pos_only, - pos_or_named, - args, - named_only, - kwargs, - } = parameter_spec_args(&x.args)?; - let pos_only: Vec = pos_only.iter().map(SignatureRegularArg::render).collect(); - let pos_or_named: Vec = pos_or_named - .iter() - .map(SignatureRegularArg::render) - .collect(); - let named_only: Vec = named_only.iter().map(SignatureRegularArg::render).collect(); + match &x.args { + RegularParams::Arguments(_) => Ok(syn::parse_quote! { + starlark::__derive_refs::sig::parameter_spec_for_arguments(#name_str) + }), + RegularParams::Unpack(args) => { + let ParametersSpecArgs { + pos_only, + pos_or_named, + args, + named_only, + kwargs, + } = parameter_spec_args(args)?; + + let pos_only: Vec = + pos_only.iter().map(SignatureRegularArg::render).collect(); + let pos_or_named: Vec = pos_or_named + .iter() + .map(SignatureRegularArg::render) + .collect(); + let named_only: Vec = + named_only.iter().map(SignatureRegularArg::render).collect(); - Ok(syn::parse_quote! { - starlark::__derive_refs::sig::parameter_spec( - #name_str, - &[#(#pos_only),*], - &[#(#pos_or_named),*], - #args, - &[#(#named_only),*], - #kwargs, - ) - }) + Ok(syn::parse_quote! { + starlark::__derive_refs::sig::parameter_spec( + #name_str, + &[#(#pos_only),*], + &[#(#pos_or_named),*], + #args, + &[#(#named_only),*], + #kwargs, + ) + }) + } + } } fn render_none() -> syn::Expr { @@ -583,58 +584,61 @@ fn render_native_callable_components(x: &StarFun) -> syn::Result { None => quote!(None), }; - let param_spec: syn::Expr = if x.is_arguments() { - syn::parse_quote! { - starlark::__derive_refs::param_spec::NativeCallableParamSpec::for_arguments() - } - } else { - let ParamSpec { - pos_only, - pos_or_named, - args, - named_only, - kwargs, - } = ParamSpec::split(&x.args)?; - - let pos_only: Vec = pos_only - .iter() - .copied() - .map(render_regular_native_callable_param) - .collect::>>()?; - let pos_or_named: Vec = pos_or_named - .iter() - .copied() - .map(render_regular_native_callable_param) - .collect::>>()?; - let args: Option = args.map(|arg| { - let name_str = ident_string(&arg.name); - let ty = render_starlark_type(&arg.ty); - syn::parse_quote! { - starlark::__derive_refs::param_spec::NativeCallableParam::args(#name_str, #ty) - } - }); - let named_only: Vec = named_only - .iter() - .copied() - .map(render_regular_native_callable_param) - .collect::>>()?; - let kwargs: Option = kwargs.map(|arg| { - let name_str = ident_string(&arg.name); - let ty = render_starlark_type(&arg.ty); + let param_spec: syn::Expr = match &x.args { + RegularParams::Arguments(_) => { syn::parse_quote! { - starlark::__derive_refs::param_spec::NativeCallableParam::kwargs(#name_str, #ty) + starlark::__derive_refs::param_spec::NativeCallableParamSpec::for_arguments() } - }); + } + RegularParams::Unpack(args) => { + let ParamSpec { + pos_only, + pos_or_named, + args, + named_only, + kwargs, + } = ParamSpec::split(args)?; - let args = render_option(args); - let kwargs = render_option(kwargs); - syn::parse_quote! { - starlark::__derive_refs::param_spec::NativeCallableParamSpec { - pos_only: vec![#(#pos_only),*], - pos_or_named: vec![#(#pos_or_named),*], - args: #args, - named_only: vec![#(#named_only),*], - kwargs: #kwargs, + let pos_only: Vec = pos_only + .iter() + .copied() + .map(render_regular_native_callable_param) + .collect::>>()?; + let pos_or_named: Vec = pos_or_named + .iter() + .copied() + .map(render_regular_native_callable_param) + .collect::>>()?; + let args: Option = args.map(|arg| { + let name_str = ident_string(&arg.name); + let ty = render_starlark_type(&arg.ty); + syn::parse_quote! { + starlark::__derive_refs::param_spec::NativeCallableParam::args(#name_str, #ty) + } + }); + let named_only: Vec = named_only + .iter() + .copied() + .map(render_regular_native_callable_param) + .collect::>>()?; + let kwargs: Option = kwargs.map(|arg| { + let name_str = ident_string(&arg.name); + let ty = render_starlark_type(&arg.ty); + syn::parse_quote! { + starlark::__derive_refs::param_spec::NativeCallableParam::kwargs(#name_str, #ty) + } + }); + + let args = render_option(args); + let kwargs = render_option(kwargs); + syn::parse_quote! { + starlark::__derive_refs::param_spec::NativeCallableParamSpec { + pos_only: vec![#(#pos_only),*], + pos_or_named: vec![#(#pos_or_named),*], + args: #args, + named_only: vec![#(#named_only),*], + kwargs: #kwargs, + } } } }; diff --git a/starlark_derive/src/module/typ.rs b/starlark_derive/src/module/typ.rs index e7626b760..3b893ef85 100644 --- a/starlark_derive/src/module/typ.rs +++ b/starlark_derive/src/module/typ.rs @@ -70,7 +70,7 @@ pub(crate) struct StarFun { pub as_type: Option, pub attrs: Vec, pub this: Option, - pub args: Vec, + pub args: RegularParams, /// Has `&Heap` parameter. pub heap: Option, /// Has `&mut Evaluator` parameter. @@ -91,12 +91,6 @@ impl StarFun { self.this.is_some() } - pub(crate) fn is_arguments(&self) -> bool { - self.args - .iter() - .any(|arg| matches!(arg.pass_style, StarArgPassStyle::Arguments)) - } - pub(crate) fn span(&self) -> Span { self.name .span() @@ -132,8 +126,6 @@ pub(crate) enum StarArgPassStyle { Args, /// `**kwargs`. Kwargs, - /// `&Arguments`. - Arguments, } /// Method `this` parameter, always first. @@ -172,10 +164,33 @@ pub(crate) struct StarArg { pub source: StarArgSource, } +/// `&Arguments` parameter. +#[derive(Debug)] +pub(crate) struct StarArguments { + pub(crate) other_attrs: Vec, + pub(crate) mutability: Option, + pub(crate) ident: syn::Ident, + pub(crate) ty: syn::Type, +} + +impl StarArguments { + pub(crate) fn reconstruct_param(&self) -> syn::PatType { + pat_type(&self.other_attrs, self.mutability, &self.ident, &self.ty) + } +} + +/// How we handle `&Arguments`. +#[derive(Debug)] +pub(crate) enum RegularParams { + /// Pass `&Arguments` as is. + Arguments(StarArguments), + /// Unpack the `&Arguments` into a multiple typed parameters. + Unpack(Vec), +} + #[derive(Debug, PartialEq, Clone)] pub(crate) enum StarArgSource { Unknown, - Parameters, Argument(usize), Required(usize), Optional(usize),