Improve C-variadic error messages: part 2

compiler/rustc_ast/src/ast.rs

@@ -2284,6 +2284,54 @@ 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(extern_span) = self.header.ext.span() {
+                    return extern_span.shrink_to_lo();
+                }
+
+                // Insert right at the front of the signature.
+                self.header_span().shrink_to_hi()
+            }
+        }
+    }
+
+    /// The span of the header's extern, or where to insert it if empty.
+    pub fn extern_span(&self) -> Span {
+        self.header.ext.span().unwrap_or(self.safety_span().shrink_to_hi())
+    }
+}
+
 /// A constraint on an associated item.
 ///
 /// ### Examples
@@ -3526,6 +3574,13 @@ impl Extern {
             None => Extern::Implicit(span),
         }
     }
+
+    pub fn span(self) -> Option<Span> {
+        match self {
+            Extern::None => None,
+            Extern::Implicit(span) | Extern::Explicit(_, span) => Some(span),
+        }
+    }
 }
 
 /// A function header.
@@ -3534,12 +3589,12 @@ impl Extern {
 /// included in this struct (e.g., `async unsafe fn` or `const extern "C" fn`).
 #[derive(Clone, Copy, Encodable, Decodable, Debug, Walkable)]
 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,
 }
@@ -3553,38 +3608,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

@@ -57,15 +57,26 @@ 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 = defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
-
 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
+    .help = only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
+
 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
@@ -84,6 +95,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

@@ -492,7 +492,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 { " " };
 
@@ -685,22 +685,53 @@ impl<'a> AstValidator<'a> {
             });
         }
 
+        if let Some(coroutine_kind) = sig.header.coroutine_kind {
+            self.dcx().emit_err(errors::CoroutineAndCVariadic {
+                spans: vec![coroutine_kind.span(), variadic_param.span],
+                coroutine_kind: coroutine_kind.as_str(),
+                coroutine_span: coroutine_kind.span(),
+                variadic_span: variadic_param.span,
+            });
+        }
+
         match fn_ctxt {
             FnCtxt::Foreign => return,
             FnCtxt::Free => match sig.header.ext {
-                Extern::Explicit(StrLit { symbol_unescaped: sym::C, .. }, _)
-                | Extern::Explicit(StrLit { symbol_unescaped: sym::C_dash_unwind, .. }, _)
-                | Extern::Implicit(_)
-                    if matches!(sig.header.safety, Safety::Unsafe(_)) =>
-                {
-                    return;
+                Extern::Implicit(_) => {
+                    if !matches!(sig.header.safety, Safety::Unsafe(_)) {
+                        self.dcx().emit_err(errors::CVariadicMustBeUnsafe {
+                            span: variadic_param.span,
+                            unsafe_span: sig.safety_span(),
+                        });
+                    }
                 }
-                _ => {}
-            },
-            FnCtxt::Assoc(_) => {}
-        };
+                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(),
+                        });
+                    }
 
-        self.dcx().emit_err(errors::BadCVariadic { span: variadic_param.span });
+                    if !matches!(sig.header.safety, Safety::Unsafe(_)) {
+                        self.dcx().emit_err(errors::CVariadicMustBeUnsafe {
+                            span: variadic_param.span,
+                            unsafe_span: sig.safety_span(),
+                        });
+                    }
+                }
+                Extern::None => {
+                    let err = errors::CVariadicNoExtern { span: variadic_param.span };
+                    self.dcx().emit_err(err);
+                }
+            },
+            FnCtxt::Assoc(_) => {
+                // For now, C variable argument lists are unsupported in associated functions.
+                let err = errors::CVariadicAssociatedFunction { span: variadic_param.span };
+                self.dcx().emit_err(err);
+            }
+        }
     }
 
     fn check_item_named(&self, ident: Ident, kind: &str) {

compiler/rustc_ast_passes/src/errors.rs

@@ -319,12 +319,46 @@ 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: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_no_extern)]
+#[help]
+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)]
+#[diag(ast_passes_c_variadic_bad_extern)]
+#[help]
+pub(crate) struct CVariadicBadExtern {
+    #[primary_span]
+    pub span: Span,
+    pub abi: Symbol,
+    #[label]
+    pub extern_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_item_underscore)]
 pub(crate) struct ItemUnderscore<'a> {
@@ -659,6 +693,18 @@ pub(crate) struct ConstAndCVariadic {
     pub variadic_span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_coroutine_and_c_variadic)]
+pub(crate) struct CoroutineAndCVariadic {
+    #[primary_span]
+    pub spans: Vec<Span>,
+    pub coroutine_kind: &'static str,
+    #[label(ast_passes_const)]
+    pub coroutine_span: Span,
+    #[label(ast_passes_variadic)]
+    pub variadic_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_pattern_in_foreign, code = E0130)]
 // FIXME: deduplicate with rustc_lint (`BuiltinLintDiag::PatternsInFnsWithoutBody`)

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

@@ -13,6 +13,6 @@ 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 defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
+    //~| ERROR `...` is not supported for non-extern functions
     //~| ERROR cannot find type `F` in this scope
 }

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

@@ -46,11 +46,13 @@ error: `...` must be the last argument of a C-variadic function
 LL |     self , ... ,   self ,   self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
    |            ^^^
 
-error: defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
+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 ) {
    |                                    ^^^
+   |
+   = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
 error[E0412]: cannot find type `F` in this scope
   --> $DIR/issue-86053-1.rs:11:48

tests/ui/c-variadic/not-async.rs

@@ -0,0 +1,7 @@
+//@ edition: 2021
+#![feature(c_variadic)]
+#![crate_type = "lib"]
+
+async unsafe extern "C" fn cannot_be_async(x: isize, ...) {}
+//~^ ERROR functions cannot be both `async` and C-variadic
+//~| ERROR hidden type for `impl Future<Output = ()>` captures lifetime that does not appear in bounds

tests/ui/c-variadic/not-async.stderr

@@ -0,0 +1,19 @@
+error: functions cannot be both `async` and C-variadic
+  --> $DIR/not-async.rs:5:1
+   |
+LL | async unsafe extern "C" fn cannot_be_async(x: isize, ...) {}
+   | ^^^^^ `async` because of this                        ^^^ C-variadic because of this
+
+error[E0700]: hidden type for `impl Future<Output = ()>` captures lifetime that does not appear in bounds
+  --> $DIR/not-async.rs:5:59
+   |
+LL | async unsafe extern "C" fn cannot_be_async(x: isize, ...) {}
+   | --------------------------------------------------------- ^^
+   | |
+   | opaque type defined here
+   |
+   = note: hidden type `{async fn body of cannot_be_async()}` captures lifetime `'_`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0700`.

tests/ui/cmse-nonsecure/cmse-nonsecure-entry/generics.rs

@@ -53,24 +53,20 @@ extern "cmse-nonsecure-entry" fn trait_object(x: &dyn Trait) -> &dyn Trait {
     x
 }
 
-extern "cmse-nonsecure-entry" fn static_trait_object(
-    x: &'static dyn Trait,
-) -> &'static dyn Trait {
+extern "cmse-nonsecure-entry" fn static_trait_object(x: &'static dyn Trait) -> &'static dyn Trait {
     //~^ ERROR return value of `"cmse-nonsecure-entry"` function too large to pass via registers [E0798]
     x
 }
 
 #[repr(transparent)]
 struct WrapperTransparent<'a>(&'a dyn Trait);
 
-extern "cmse-nonsecure-entry" fn wrapped_trait_object(
-    x: WrapperTransparent,
-) -> WrapperTransparent {
+extern "cmse-nonsecure-entry" fn wrapped_trait_object(x: WrapperTransparent) -> WrapperTransparent {
     //~^ ERROR return value of `"cmse-nonsecure-entry"` function too large to pass via registers [E0798]
     x
 }
 
-extern "cmse-nonsecure-entry" fn c_variadic(_: u32, _: ...) {
-    //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
+unsafe extern "cmse-nonsecure-entry" fn c_variadic(_: u32, _: ...) {
+    //~^ ERROR `...` is not supported for `extern "cmse-nonsecure-entry"` functions
     //~| ERROR requires `va_list` lang_item
 }