improve c-variadic errors

compiler/rustc_ast/src/ast.rs

@@ -2227,6 +2227,62 @@ pub struct FnSig {
     pub span: Span,
 }
 
+impl FnSig {
+    /// Return a span encompassing the header, or where to insert it if empty.
+    pub fn header_span(&self) -> Span {
+        match self.header.ext {
+            Extern::Implicit(span) | Extern::Explicit(_, span) => {
+                return self.span.with_hi(span.hi());
+            }
+            Extern::None => {}
+        }
+
+        match self.header.safety {
+            Safety::Unsafe(span) | Safety::Safe(span) => return self.span.with_hi(span.hi()),
+            Safety::Default => {}
+        };
+
+        if let Some(coroutine_kind) = self.header.coroutine_kind {
+            return self.span.with_hi(coroutine_kind.span().hi());
+        }
+
+        if let Const::Yes(span) = self.header.constness {
+            return self.span.with_hi(span.hi());
+        }
+
+        self.span.shrink_to_lo()
+    }
+
+    /// The span of the header's safety, or where to insert it if empty.
+    pub fn safety_span(&self) -> Span {
+        match self.header.safety {
+            Safety::Unsafe(span) | Safety::Safe(span) => span,
+            Safety::Default => {
+                // Insert after the `coroutine_kind` if available.
+                if let Some(coroutine_kind) = self.header.coroutine_kind {
+                    return coroutine_kind.span().shrink_to_hi();
+                }
+
+                // Insert after the `const` keyword if available.
+                if let Const::Yes(const_span) = self.header.constness {
+                    return const_span.shrink_to_hi();
+                }
+
+                // Insert right at the front of the signature.
+                self.span.shrink_to_lo()
+            }
+        }
+    }
+
+    /// The span of the header's extern, or where to insert it if empty.
+    pub fn extern_span(&self) -> Span {
+        match self.header.ext {
+            Extern::Implicit(span) | Extern::Explicit(_, span) => span,
+            Extern::None => self.safety_span().shrink_to_hi(),
+        }
+    }
+}
+
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 #[derive(Encodable, Decodable, HashStable_Generic)]
 pub enum FloatTy {
@@ -3571,12 +3627,12 @@ impl Extern {
 /// included in this struct (e.g., `async unsafe fn` or `const extern "C" fn`).
 #[derive(Clone, Copy, Encodable, Decodable, Debug)]
 pub struct FnHeader {
-    /// Whether this is `unsafe`, or has a default safety.
-    pub safety: Safety,
-    /// Whether this is `async`, `gen`, or nothing.
-    pub coroutine_kind: Option<CoroutineKind>,
     /// The `const` keyword, if any
     pub constness: Const,
+    /// Whether this is `async`, `gen`, or nothing.
+    pub coroutine_kind: Option<CoroutineKind>,
+    /// Whether this is `unsafe`, or has a default safety.
+    pub safety: Safety,
     /// The `extern` keyword and corresponding ABI string, if any.
     pub ext: Extern,
 }
@@ -3590,38 +3646,6 @@ impl FnHeader {
             || matches!(constness, Const::Yes(_))
             || !matches!(ext, Extern::None)
     }
-
-    /// Return a span encompassing the header, or none if all options are default.
-    pub fn span(&self) -> Option<Span> {
-        fn append(a: &mut Option<Span>, b: Span) {
-            *a = match a {
-                None => Some(b),
-                Some(x) => Some(x.to(b)),
-            }
-        }
-
-        let mut full_span = None;
-
-        match self.safety {
-            Safety::Unsafe(span) | Safety::Safe(span) => append(&mut full_span, span),
-            Safety::Default => {}
-        };
-
-        if let Some(coroutine_kind) = self.coroutine_kind {
-            append(&mut full_span, coroutine_kind.span());
-        }
-
-        if let Const::Yes(span) = self.constness {
-            append(&mut full_span, span);
-        }
-
-        match self.ext {
-            Extern::Implicit(span) | Extern::Explicit(_, span) => append(&mut full_span, span),
-            Extern::None => {}
-        }
-
-        full_span
-    }
 }
 
 impl Default for FnHeader {

compiler/rustc_ast_passes/messages.ftl

@@ -46,15 +46,25 @@ ast_passes_auto_super_lifetime = auto traits cannot have super traits or lifetim
     .label = {ast_passes_auto_super_lifetime}
     .suggestion = remove the super traits or lifetime bounds
 
-ast_passes_bad_c_variadic = only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
-
 ast_passes_body_in_extern = incorrect `{$kind}` inside `extern` block
     .cannot_have = cannot have a body
     .invalid = the invalid body
     .existing = `extern` blocks define existing foreign {$kind}s and {$kind}s inside of them cannot have a body
 
 ast_passes_bound_in_context = bounds on `type`s in {$ctx} have no effect
 
+ast_passes_c_variadic_associated_function = associated functions cannot have a C variable argument list
+
+ast_passes_c_variadic_bad_extern = `...` is not supported for `extern "{$abi}"` functions
+    .label = `extern "{$abi}"` because of this
+    .help = only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
+
+ast_passes_c_variadic_must_be_unsafe =
+    functions with a C variable argument list must be unsafe
+    .suggestion = add the `unsafe` keyword to this definition
+
+ast_passes_c_variadic_no_extern = `...` is not supported for non-extern functions
+
 ast_passes_const_and_c_variadic = functions cannot be both `const` and C-variadic
     .const = `const` because of this
     .variadic = C-variadic because of this
@@ -73,6 +83,10 @@ ast_passes_const_without_body =
 ast_passes_constraint_on_negative_bound =
     associated type constraints not allowed on negative bounds
 
+ast_passes_coroutine_and_c_variadic = functions cannot be both `{$coroutine_kind}` and C-variadic
+    .const = `{$coroutine_kind}` because of this
+    .variadic = C-variadic because of this
+
 ast_passes_equality_in_where = equality constraints are not yet supported in `where` clauses
     .label = not supported
     .suggestion = if `{$ident}` is an associated type you're trying to set, use the associated type binding syntax

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -460,7 +460,7 @@ impl<'a> AstValidator<'a> {
         }
 
         if !spans.is_empty() {
-            let header_span = sig.header.span().unwrap_or(sig.span.shrink_to_lo());
+            let header_span = sig.header_span();
             let suggestion_span = header_span.shrink_to_hi().to(sig.decl.output.span());
             let padding = if header_span.is_empty() { "" } else { " " };
 
@@ -633,46 +633,73 @@ impl<'a> AstValidator<'a> {
     /// - Non-const
     /// - Either foreign, or free and `unsafe extern "C"` semantically
     fn check_c_variadic_type(&self, fk: FnKind<'a>) {
-        let variadic_spans: Vec<_> = fk
-            .decl()
-            .inputs
-            .iter()
-            .filter(|arg| matches!(arg.ty.kind, TyKind::CVarArgs))
-            .map(|arg| arg.span)
-            .collect();
+        let variadic_params: Vec<_> =
+            fk.decl().inputs.iter().filter(|arg| matches!(arg.ty.kind, TyKind::CVarArgs)).collect();
 
-        if variadic_spans.is_empty() {
+        // The parser already rejects `...` if it's not the final argument, but we still want to
+        // emit the errors below, so we only consider the final `...` here.
+        let Some(variadic_param) = variadic_params.last() else {
             return;
+        };
+
+        let FnKind::Fn(fn_ctxt, _, Fn { sig, .. }) = fk else {
+            // Unreachable because the parser already rejects `...` in closures.
+            unreachable!("C variable argument list cannot be used in closures")
+        };
+
+        // C-variadics are not yet implemented in const evaluation.
+        if let Const::Yes(const_span) = sig.header.constness {
+            self.dcx().emit_err(errors::ConstAndCVariadic {
+                span: const_span.to(variadic_param.span),
+                const_span,
+                variadic_span: variadic_param.span,
+            });
         }
 
-        if let Some(header) = fk.header() {
-            if let Const::Yes(const_span) = header.constness {
-                let mut spans = variadic_spans.clone();
-                spans.push(const_span);
-                self.dcx().emit_err(errors::ConstAndCVariadic {
-                    spans,
-                    const_span,
-                    variadic_spans: variadic_spans.clone(),
-                });
-            }
+        if let Some(coroutine_kind) = sig.header.coroutine_kind {
+            self.dcx().emit_err(errors::CoroutineAndCVariadic {
+                span: coroutine_kind.span().to(variadic_param.span),
+                coroutine_kind: coroutine_kind.as_str(),
+                coroutine_span: coroutine_kind.span(),
+                variadic_span: variadic_param.span,
+            });
         }
 
-        match (fk.ctxt(), fk.header()) {
-            (Some(FnCtxt::Foreign), _) => return,
-            (Some(FnCtxt::Free), Some(header)) => match header.ext {
-                Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }, _)
-                | Extern::Explicit(StrLit { symbol_unescaped: sym::C_dash_unwind, .. }, _)
-                | Extern::Implicit(_)
-                    if matches!(header.safety, Safety::Unsafe(_)) =>
-                {
-                    return;
-                }
-                _ => {}
-            },
-            _ => {}
-        };
+        match fn_ctxt {
+            FnCtxt::Free => {
+                match sig.header.ext {
+                    Extern::Implicit(_) => { /* defaults to "C" */ }
+                    Extern::Explicit(StrLit { symbol_unescaped, .. }, _) => {
+                        if !matches!(symbol_unescaped, sym::C | sym::C_dash_unwind) {
+                            self.dcx().emit_err(errors::CVariadicBadExtern {
+                                span: variadic_param.span,
+                                abi: symbol_unescaped,
+                                extern_span: sig.extern_span(),
+                            });
+                        }
+                    }
+                    Extern::None => {
+                        self.dcx()
+                            .emit_err(errors::CVariadicNoExtern { span: variadic_param.span });
+                    }
+                };
 
-        self.dcx().emit_err(errors::BadCVariadic { span: variadic_spans });
+                if !matches!(sig.header.safety, Safety::Unsafe(_)) {
+                    self.dcx().emit_err(errors::CVariadicMustBeUnsafe {
+                        span: variadic_param.span,
+                        unsafe_span: sig.safety_span(),
+                    });
+                }
+            }
+            FnCtxt::Assoc(_) => {
+                // For now, C variable argument lists are unsupported in associated functions.
+                self.dcx()
+                    .emit_err(errors::CVariadicAssociatedFunction { span: variadic_param.span });
+            }
+            FnCtxt::Foreign => {
+                // Whether the ABI supports C variable argument lists is checked later.
+            }
+        }
     }
 
     fn check_item_named(&self, ident: Ident, kind: &str) {

compiler/rustc_ast_passes/src/errors.rs

@@ -309,10 +309,42 @@ pub(crate) struct ExternItemAscii {
 }
 
 #[derive(Diagnostic)]
-#[diag(ast_passes_bad_c_variadic)]
-pub(crate) struct BadCVariadic {
+#[diag(ast_passes_c_variadic_associated_function)]
+pub(crate) struct CVariadicAssociatedFunction {
     #[primary_span]
-    pub span: Vec<Span>,
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_bad_extern)]
+pub(crate) struct CVariadicBadExtern {
+    #[primary_span]
+    pub span: Span,
+    pub abi: Symbol,
+    #[label]
+    pub extern_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_no_extern)]
+pub(crate) struct CVariadicNoExtern {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_must_be_unsafe)]
+pub(crate) struct CVariadicMustBeUnsafe {
+    #[primary_span]
+    pub span: Span,
+
+    #[suggestion(
+        ast_passes_suggestion,
+        applicability = "maybe-incorrect",
+        code = "unsafe ",
+        style = "verbose"
+    )]
+    pub unsafe_span: Span,
 }
 
 #[derive(Diagnostic)]
@@ -663,11 +695,23 @@ pub(crate) struct ConstAndCoroutine {
 #[diag(ast_passes_const_and_c_variadic)]
 pub(crate) struct ConstAndCVariadic {
     #[primary_span]
-    pub spans: Vec<Span>,
+    pub span: Span,
     #[label(ast_passes_const)]
     pub const_span: Span,
     #[label(ast_passes_variadic)]
-    pub variadic_spans: Vec<Span>,
+    pub variadic_span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_coroutine_and_c_variadic)]
+pub(crate) struct CoroutineAndCVariadic {
+    #[primary_span]
+    pub span: Span,
+    pub coroutine_kind: &'static str,
+    #[label(ast_passes_const)]
+    pub coroutine_span: Span,
+    #[label(ast_passes_variadic)]
+    pub variadic_span: Span,
 }
 
 #[derive(Diagnostic)]

tests/ui/c-variadic/issue-86053-1.rs

@@ -13,6 +13,7 @@ fn ordering4 < 'a , 'b     > ( a :            ,   self , self ,   self ,
     //~| ERROR unexpected `self` parameter in function
     //~| ERROR unexpected `self` parameter in function
     //~| ERROR `...` must be the last argument of a C-variadic function
-    //~| ERROR only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
+    //~| ERROR `...` is not supported for non-extern functions
+    //~| ERROR: functions with a C variable argument list must be unsafe
     //~| ERROR cannot find type `F` in this scope
 }

tests/ui/c-variadic/issue-86053-1.stderr

@@ -46,11 +46,22 @@ error: `...` must be the last argument of a C-variadic function
 LL |     self , ... ,   self ,   self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
    |            ^^^
 
-error: only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
-  --> $DIR/issue-86053-1.rs:11:12
+error: `...` is not supported for non-extern functions
+  --> $DIR/issue-86053-1.rs:11:36
+   |
+LL |     self , ... ,   self ,   self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
+   |                                    ^^^
+
+error: functions with a C variable argument list must be unsafe
+  --> $DIR/issue-86053-1.rs:11:36
    |
 LL |     self , ... ,   self ,   self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
-   |            ^^^                     ^^^
+   |                                    ^^^
+   |
+help: add the `unsafe` keyword to this definition
+   |
+LL | unsafe fn ordering4 < 'a , 'b     > ( a :            ,   self , self ,   self ,
+   | ++++++
 
 error[E0412]: cannot find type `F` in this scope
   --> $DIR/issue-86053-1.rs:11:48
@@ -70,6 +81,6 @@ help: you might be missing a type parameter
 LL | fn ordering4 < 'a , 'b, F     > ( a :            ,   self , self ,   self ,
    |                       +++
 
-error: aborting due to 10 previous errors
+error: aborting due to 11 previous errors
 
 For more information about this error, try `rustc --explain E0412`.

tests/ui/c-variadic/issue-86053-2.rs

@@ -5,7 +5,7 @@
 
 trait H<T> {}
 
-unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), ...) {}
+unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), _: ...) {}
 //~^ ERROR: in type `&'static &'a ()`, reference has a longer lifetime than the data it references [E0491]
 
 fn main() {}