improve c-variadic errors

Author: folkertdev

compiler/rustc_ast/src/ast.rs

@@ -2309,6 +2309,35 @@ impl FnSig {
 
         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(),
+        }
+    }
 }
 
 /// A constraint on an associated item.

compiler/rustc_ast_passes/messages.ftl

@@ -57,15 +57,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 = 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
+
 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 +94,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

@@ -665,46 +665,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;
-        }
+        };
 
-        if let Some(header) = fk.header()
-            && let Const::Yes(const_span) = header.constness
-        {
-            let mut spans = variadic_spans.clone();
-            spans.push(const_span);
+        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 {
-                spans,
+                span: const_span.to(variadic_param.span),
                 const_span,
-                variadic_spans: variadic_spans.clone(),
+                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;
-                }
-                _ => {}
-            },
-            _ => {}
-        };
+        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,
+            });
+        }
 
-        self.dcx().emit_err(errors::BadCVariadic { span: variadic_spans });
+        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 });
+                    }
+                };
+
+                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

@@ -319,10 +319,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)]
@@ -652,11 +684,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 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: 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: defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
-  --> $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() {}

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

@@ -1,14 +1,14 @@
 error[E0491]: in type `&'static &'a ()`, reference has a longer lifetime than the data it references
   --> $DIR/issue-86053-2.rs:8:39
    |
-LL | unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), ...) {}
+LL | unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), _: ...) {}
    |                                       ^^^^^^^^^^^^^^^^^^
    |
    = note: the pointer is valid for the static lifetime
 note: but the referenced data is only valid for the lifetime `'a` as defined here
   --> $DIR/issue-86053-2.rs:8:32
    |
-LL | unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), ...) {}
+LL | unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), _: ...) {}
    |                                ^^
 
 error: aborting due to 1 previous error

tests/ui/c-variadic/no-associated-function.rs

@@ -0,0 +1,42 @@
+// For now C-variadic arguments in associated functions are rejected. Especially when `self`
+// parameters are used there may be weird interactions with other compiler features. We may
+// relax this restriction in the future.
+#![feature(c_variadic)]
+#![crate_type = "lib"]
+struct S;
+
+impl S {
+    unsafe extern "C" fn associated_function(mut ap: ...) -> i32 {
+        //~^ ERROR: associated functions cannot have a C variable argument list
+        unsafe { ap.arg() }
+    }
+
+    unsafe extern "C" fn method(&self, mut ap: ...) -> i32 {
+        //~^ ERROR: associated functions cannot have a C variable argument list
+        unsafe { ap.arg() }
+    }
+}
+
+trait T {
+    unsafe extern "C" fn trait_associated_function(mut ap: ...) -> i32 {
+        //~^ ERROR: associated functions cannot have a C variable argument list
+        unsafe { ap.arg() }
+    }
+
+    unsafe extern "C" fn trait_method(&self, mut ap: ...) -> i32 {
+        //~^ ERROR: associated functions cannot have a C variable argument list
+        unsafe { ap.arg() }
+    }
+}
+
+impl T for S {}
+
+fn main() {
+    unsafe {
+        assert_eq!(S::associated_function(32), 32);
+        assert_eq!(S.method(32), 32);
+
+        assert_eq!(S::trait_associated_function(32), 32);
+        assert_eq!(S.trait_method(32), 32);
+    }
+}

tests/ui/c-variadic/no-associated-function.stderr

@@ -0,0 +1,26 @@
+error: associated functions cannot have a C variable argument list
+  --> $DIR/no-associated-function.rs:9:46
+   |
+LL |     unsafe extern "C" fn associated_function(mut ap: ...) -> i32 {
+   |                                              ^^^^^^^^^^^
+
+error: associated functions cannot have a C variable argument list
+  --> $DIR/no-associated-function.rs:14:40
+   |
+LL |     unsafe extern "C" fn method(&self, mut ap: ...) -> i32 {
+   |                                        ^^^^^^^^^^^
+
+error: associated functions cannot have a C variable argument list
+  --> $DIR/no-associated-function.rs:21:52
+   |
+LL |     unsafe extern "C" fn trait_associated_function(mut ap: ...) -> i32 {
+   |                                                    ^^^^^^^^^^^
+
+error: associated functions cannot have a C variable argument list
+  --> $DIR/no-associated-function.rs:26:46
+   |
+LL |     unsafe extern "C" fn trait_method(&self, mut ap: ...) -> i32 {
+   |                                              ^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+