-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Add arg splat experiment initial tuple impl #153697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
108b87f
f6ad19f
9eac82b
571f4fc
ce1b06e
5fcfb4e
a28d527
84cab65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,7 +350,8 @@ impl<'a> AstValidator<'a> { | |
|
|
||
| fn check_fn_decl(&self, fn_decl: &FnDecl, self_semantic: SelfSemantic) { | ||
| self.check_decl_num_args(fn_decl); | ||
| self.check_decl_cvariadic_pos(fn_decl); | ||
| let c_variadic_span = self.check_decl_cvariadic_pos(fn_decl); | ||
| self.check_decl_splatting(fn_decl, c_variadic_span); | ||
| self.check_decl_attrs(fn_decl); | ||
| self.check_decl_self_param(fn_decl, self_semantic); | ||
| } | ||
|
|
@@ -368,17 +369,68 @@ impl<'a> AstValidator<'a> { | |
| /// Emits an error if a function declaration has a variadic parameter in the | ||
| /// beginning or middle of parameter list. | ||
| /// Example: `fn foo(..., x: i32)` will emit an error. | ||
| fn check_decl_cvariadic_pos(&self, fn_decl: &FnDecl) { | ||
| /// Returns true if a C-variadic parameter is found. | ||
| fn check_decl_cvariadic_pos(&self, fn_decl: &FnDecl) -> Option<Span> { | ||
| let mut c_variadic_span = None; | ||
|
|
||
| match &*fn_decl.inputs { | ||
| [ps @ .., _] => { | ||
| for Param { ty, span, .. } in ps { | ||
| if let TyKind::CVarArgs = ty.kind { | ||
| c_variadic_span = Some(*span); | ||
| self.dcx().emit_err(errors::FnParamCVarArgsNotLast { span: *span }); | ||
| } | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
|
|
||
| if let Some(Param { ty, span, .. }) = &fn_decl.inputs.last() { | ||
| if let TyKind::CVarArgs = ty.kind { | ||
| c_variadic_span = Some(*span); | ||
| } | ||
| } | ||
|
|
||
| c_variadic_span | ||
| } | ||
|
|
||
| /// Emits an error if a function declaration has more than one splatted argument, with a | ||
| /// C-variadic parameter, or a splat at an unsupported index (for performance). | ||
| /// Example: `fn foo(#[splat] x: (), #[splat] y: ())` will emit an error. | ||
| fn check_decl_splatting(&self, fn_decl: &FnDecl, c_variadic_span: Option<Span>) { | ||
| let (splatted_arg_indexes, mut splatted_spans): (Vec<u16>, Vec<Span>) = fn_decl | ||
| .inputs | ||
| .iter() | ||
| .enumerate() | ||
| .filter_map(|(index, arg)| { | ||
| arg.attrs | ||
| .iter() | ||
| .any(|attr| attr.has_name(sym::splat)) | ||
| .then_some((u16::try_from(index).unwrap(), arg.span)) | ||
| }) | ||
| .unzip(); | ||
|
|
||
| // A splatted argument at the "no splatted" marker index is not supported (this is an | ||
| // unlikely edge case). | ||
| if let (Some(&splatted_arg_index), Some(&splatted_span)) = | ||
| (splatted_arg_indexes.last(), splatted_spans.last()) | ||
| && splatted_arg_index == FnDecl::NO_SPLATTED_ARG_INDEX | ||
| { | ||
| self.dcx() | ||
| .emit_err(errors::InvalidSplattedArg { splatted_arg_index, span: splatted_span }); | ||
| } | ||
|
|
||
| // Multiple splatted arguments are invalid: we can't know which arguments go in each splat. | ||
| if splatted_arg_indexes.len() > 1 { | ||
| self.dcx().emit_err(errors::DuplicateSplattedArgs { spans: splatted_spans.clone() }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm somewhat worried that due to these errors being non-blocking for the rest of the compiler, that it's possible to cause weird ICEs, but that's nothing new wrt most of the ast validation checks. |
||
| } | ||
|
|
||
| if let Some(c_variadic_span) = c_variadic_span | ||
| && !splatted_spans.is_empty() | ||
| { | ||
| splatted_spans.push(c_variadic_span); | ||
| self.dcx().emit_err(errors::CVarArgsAndSplat { spans: splatted_spans }); | ||
| } | ||
| } | ||
|
|
||
| fn check_decl_attrs(&self, fn_decl: &FnDecl) { | ||
|
|
@@ -394,6 +446,7 @@ impl<'a> AstValidator<'a> { | |
| sym::deny, | ||
| sym::expect, | ||
| sym::forbid, | ||
| sym::splat, | ||
| sym::warn, | ||
| ]; | ||
| !attr.has_any_name(&arr) && rustc_attr_parsing::is_builtin_attr(*attr) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| //! Attribute parsing for the `#[splat]` function argument overloading attribute. | ||
| //! This attribute modifies typecheck to support overload resolution, then modifies codegen for performance. | ||
|
|
||
| use super::prelude::*; | ||
|
|
||
| pub(crate) struct SplatParser; | ||
|
|
||
| impl<S: Stage> NoArgsAttributeParser<S> for SplatParser { | ||
| const PATH: &[Symbol] = &[sym::splat]; | ||
| const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Warn; | ||
| const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(&[ | ||
| Allow(Target::Param), | ||
| // FIXME(splat): only allow MacroCall if the macro creates an argument | ||
| Allow(Target::MacroCall), | ||
|
Comment on lines
+13
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could also just not allow this for now, or add a test that excercises this code path |
||
| ]); | ||
| const CREATE: fn(Span) -> AttributeKind = AttributeKind::Splat; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This return type should be a struct I think 😆 it's getting a little out of hand, and usually is just passed along to another function anyway, so we could just keep passing these modifiers along together. Also the function name doesn't really represent the return value anymore... maybe
param_info?View changes since the review