Fix splat ICEs and ban it in closures

compiler/rustc_ast/src/ast.rs

@@ -3064,6 +3064,9 @@ impl FnDecl {
     /// Must have the same value as `FnSigKind::NO_SPLATTED_ARG_INDEX` and `FnDeclFlags::NO_SPLATTED_ARG_INDEX`.
     pub const NO_SPLATTED_ARG_INDEX: u8 = u8::MAX;
 
+    /// The maximum valid splatted argument index.
+    pub const MAX_VALID_SPLATTED_ARG_INDEX: u8 = Self::NO_SPLATTED_ARG_INDEX - 1;
+
     /// Returns a splatted argument index, if any are present.
     pub fn splatted(&self) -> Option<u8> {
         self.inputs.iter().enumerate().find_map(|(index, arg)| {

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -16,6 +16,7 @@
 //! constructions produced by proc macros. This pass is only intended for simple checks that do not
 //! require name resolution or type checking, or other kinds of complex analysis.
 
+use std::collections::BTreeMap;
 use std::mem;
 use std::str::FromStr;
 
@@ -34,7 +35,7 @@ use rustc_session::lint::builtin::{
     DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, MISSING_UNSAFE_ON_EXTERN,
     PATTERNS_IN_FNS_WITHOUT_BODY, UNUSED_VISIBILITIES,
 };
-use rustc_span::{Ident, Span, kw, sym};
+use rustc_span::{Ident, Span, Symbol, kw, sym};
 use rustc_target::spec::{AbiMap, AbiMapping};
 
 use crate::diagnostics::{self, TildeConstReason};
@@ -45,6 +46,41 @@ enum SelfSemantic {
     No,
 }
 
+/// Is `#[splat]` allowed semantically in a function or closure?
+/// Only applies to the function kind and header, the parameters are checked elsewhere.
+enum SplatSemantic {
+    Yes,
+    NoClosures(Span),
+    NoAbiCall { span: Span, abi: Symbol },
+}
+
+impl SplatSemantic {
+    /// Returns if splatting is semantically allowed for the given `FnKind`,
+    /// Only checks the function kind and header, not the parameters.
+    fn from_fn_kind(fk: &FnKind<'_>) -> Self {
+        match fk {
+            FnKind::Fn(_, _, f) => Self::from_extern(f.sig.header.ext),
+            // Splatting closures is banned, because closure arguments are already de-tupled.
+            FnKind::Closure(_, _, _, expr) => SplatSemantic::NoClosures(expr.span),
+        }
+    }
+
+    fn from_extern(ext: Extern) -> Self {
+        match ext {
+            Extern::None => SplatSemantic::Yes,
+            // FIXME(splat): should splatting extern "C" or other ABIs be allowed?
+            Extern::Implicit(_) => SplatSemantic::Yes,
+            // For now, splatting rust-call is banned, because it already de-tuples args.
+            Extern::Explicit(abi_str, span) => match abi_str.symbol_unescaped {
+                sym::rust_dash_call => {
+                    SplatSemantic::NoAbiCall { span, abi: abi_str.symbol_unescaped }
+                }
+                _ => SplatSemantic::Yes,
+            },
+        }
+    }
+}
+
 enum TraitOrImpl {
     Trait { vis: Span, constness: Const },
     TraitImpl { constness: Const, polarity: ImplPolarity, trait_ref_span: Span },
@@ -350,10 +386,15 @@ impl<'a> AstValidator<'a> {
         });
     }
 
-    fn check_fn_decl(&self, fn_decl: &FnDecl, self_semantic: SelfSemantic) {
+    fn check_fn_decl(
+        &self,
+        fn_decl: &FnDecl,
+        self_semantic: SelfSemantic,
+        splat_semantic: SplatSemantic,
+    ) {
         self.check_decl_num_args(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_splatting(fn_decl, c_variadic_span, splat_semantic);
         self.check_decl_attrs(fn_decl);
         self.check_decl_self_param(fn_decl, self_semantic);
     }
@@ -399,42 +440,76 @@ impl<'a> AstValidator<'a> {
     /// 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
+    fn check_decl_splatting(
+        &self,
+        fn_decl: &FnDecl,
+        c_variadic_span: Option<Span>,
+        splat_semantic: SplatSemantic,
+    ) {
+        let mut splatted_arg_spans: BTreeMap<u16, Vec<Span>> = fn_decl
             .inputs
             .iter()
             .enumerate()
             .filter_map(|(index, arg)| {
-                arg.attrs
+                let splat_arg_spans: Vec<Span> = arg
+                    .attrs
                     .iter()
-                    .any(|attr| attr.has_name(sym::splat))
-                    .then_some((u16::try_from(index).unwrap(), arg.span))
+                    .filter_map(|attr| attr.has_name(sym::splat).then_some(attr.span))
+                    .collect();
+                if splat_arg_spans.is_empty() {
+                    None
+                } else {
+                    Some((u16::try_from(index).unwrap(), splat_arg_spans))
+                }
             })
-            .unzip();
+            .collect();
 
         // A splatted argument greater than or equal to the "no splatted" marker index is not
-        // supported.
-        if let (Some(&splatted_arg_index), Some(&splatted_span)) =
-            (splatted_arg_indexes.last(), splatted_spans.last())
-            && splatted_arg_index >= u16::from(FnDecl::NO_SPLATTED_ARG_INDEX)
-        {
-            self.dcx().emit_err(diagnostics::InvalidSplattedArg {
-                splatted_arg_index,
-                span: splatted_span,
+        // supported. It is ok to drop these spans after issuing this error, because they are
+        // always invalid.
+        let out_of_range_spans =
+            splatted_arg_spans.split_off(&u16::from(FnDecl::NO_SPLATTED_ARG_INDEX));
+        if !out_of_range_spans.is_empty() {
+            self.dcx().emit_err(diagnostics::InvalidSplattedArgs {
+                max_valid_splatted_arg_index: u16::from(FnDecl::MAX_VALID_SPLATTED_ARG_INDEX),
+                first_invalid_splatted_arg_index: *out_of_range_spans.keys().next().unwrap(),
+                spans: out_of_range_spans.values().flatten().copied().collect(),
             });
         }
 
-        // 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(diagnostics::DuplicateSplattedArgs { spans: splatted_spans.clone() });
-        }
+        if !splatted_arg_spans.is_empty() {
+            let splatted_spans = || splatted_arg_spans.values().flatten().copied().collect();
 
-        if let Some(c_variadic_span) = c_variadic_span
-            && !splatted_spans.is_empty()
-        {
-            splatted_spans.push(c_variadic_span);
-            self.dcx().emit_err(diagnostics::CVarArgsAndSplat { spans: splatted_spans });
+            // Multiple splatted arguments are invalid: we can't know which arguments go in each splat.
+            if splatted_arg_spans.len() > 1 {
+                self.dcx().emit_err(diagnostics::DuplicateSplattedArgs { spans: splatted_spans() });
+            }
+
+            // C-variadic parameters and splats are not allowed together.
+            if let Some(c_variadic_span) = c_variadic_span {
+                let mut splatted_spans = splatted_spans();
+                splatted_spans.push(c_variadic_span);
+                self.dcx().emit_err(diagnostics::CVarArgsAndSplat { spans: splatted_spans });
+            }
+
+            // Splatting is not allowed on closures, or some function ABIs.
+            match splat_semantic {
+                SplatSemantic::NoClosures(closure_span) => {
+                    let mut splatted_spans = splatted_spans();
+                    splatted_spans.push(closure_span);
+                    self.dcx()
+                        .emit_err(diagnostics::SplatNotAllowedOnClosures { spans: splatted_spans });
+                }
+                SplatSemantic::NoAbiCall { span, abi } => {
+                    let mut splatted_spans = splatted_spans();
+                    splatted_spans.push(span);
+                    self.dcx().emit_err(diagnostics::SplatNotAllowedOnAbiCall {
+                        spans: splatted_spans,
+                        abi,
+                    });
+                }
+                SplatSemantic::Yes => {}
+            }
         }
     }
 
@@ -1055,7 +1130,11 @@ impl<'a> AstValidator<'a> {
         match &ty.kind {
             TyKind::FnPtr(bfty) => {
                 self.check_fn_ptr_safety(bfty.decl_span, bfty.safety);
-                self.check_fn_decl(&bfty.decl, SelfSemantic::No);
+                self.check_fn_decl(
+                    &bfty.decl,
+                    SelfSemantic::No,
+                    SplatSemantic::from_extern(bfty.ext),
+                );
                 Self::check_decl_no_pat(&bfty.decl, |span, _, _| {
                     self.dcx().emit_err(diagnostics::PatternFnPointer { span });
                 });
@@ -1746,7 +1825,8 @@ impl Visitor<'_> for AstValidator<'_> {
             Some(FnCtxt::Assoc(_)) => SelfSemantic::Yes,
             _ => SelfSemantic::No,
         };
-        self.check_fn_decl(fk.decl(), self_semantic);
+        let splat_semantic = SplatSemantic::from_fn_kind(&fk);
+        self.check_fn_decl(fk.decl(), self_semantic, splat_semantic);
 
         if let Some(&FnHeader { safety, .. }) = fk.header() {
             self.check_item_safety(span, safety);

compiler/rustc_ast_passes/src/diagnostics.rs

@@ -124,32 +124,53 @@ pub(crate) struct FnParamCVarArgsNotLast {
 }
 
 #[derive(Diagnostic)]
-#[diag("`#[splat]` is not supported on argument index {$splatted_arg_index}")]
+#[diag(
+    "`#[splat]` is only supported on argument index {$max_valid_splatted_arg_index} or less, this `#[splat]` is on index {$first_invalid_splatted_arg_index}"
+)]
 #[help("remove `#[splat]`, or use it on an argument closer to the start of the argument list")]
-pub(crate) struct InvalidSplattedArg {
-    pub splatted_arg_index: u16,
+pub(crate) struct InvalidSplattedArgs {
+    pub max_valid_splatted_arg_index: u16,
+
+    pub first_invalid_splatted_arg_index: u16,
 
     #[primary_span]
     #[label("`#[splat]` is not supported here")]
-    pub span: Span,
+    pub spans: Vec<Span>,
 }
 
 #[derive(Diagnostic)]
-#[diag("multiple `#[splat]`s are not allowed in the same function")]
+#[diag("multiple `#[splat]`s are not allowed in the same function argument list")]
 #[help("remove `#[splat]` from all but one argument")]
 pub(crate) struct DuplicateSplattedArgs {
     #[primary_span]
     pub spans: Vec<Span>,
 }
 
 #[derive(Diagnostic)]
-#[diag("`...` and `#[splat]` are not allowed in the same function")]
+#[diag("`...` and `#[splat]` are not allowed in the same function argument list")]
 #[help("remove `#[splat]` or remove `...`")]
 pub(crate) struct CVarArgsAndSplat {
     #[primary_span]
     pub spans: Vec<Span>,
 }
 
+#[derive(Diagnostic)]
+#[diag("`#[splat]` is not allowed on closure arguments")]
+#[help("remove `#[splat]` or turn the closure into a function")]
+pub(crate) struct SplatNotAllowedOnClosures {
+    #[primary_span]
+    pub spans: Vec<Span>,
+}
+
+#[derive(Diagnostic)]
+#[diag("`#[splat]` is not allowed in the arguments of functions with the `{$abi}` ABI")]
+#[help("remove `#[splat]` or change the ABI")]
+pub(crate) struct SplatNotAllowedOnAbiCall {
+    #[primary_span]
+    pub spans: Vec<Span>,
+    pub abi: Symbol,
+}
+
 #[derive(Diagnostic)]
 #[diag("documentation comments cannot be applied to function parameters")]
 pub(crate) struct FnParamDocComment {