Auto merge of #146165 - folkertdev:c-variadic-errors-take-2, r=lcnr

improve c-variadic error reporting

tracking issue: #44930

The parts of #143546 that don't require any particular knowledge about c-variadic functions.

This prepares the way for rejecting c-variadic functions that are also coroutines, safe functions, or associated functions.

Author: bors

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -665,46 +665,42 @@ 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();
+        // `...` is already rejected when it is not the final parameter.
+        let variadic_param = match fk.decl().inputs.last() {
+            Some(param) if matches!(param.ty.kind, TyKind::CVarArgs) => param,
+            _ => return,
+        };
 
-        if variadic_spans.is_empty() {
-            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")
+        };
 
-        if let Some(header) = fk.header()
-            && let Const::Yes(const_span) = header.constness
-        {
-            let mut spans = variadic_spans.clone();
-            spans.push(const_span);
+        // 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,
+                spans: vec![const_span, 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 {
+        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!(header.safety, Safety::Unsafe(_)) =>
+                    if matches!(sig.header.safety, Safety::Unsafe(_)) =>
                 {
                     return;
                 }
                 _ => {}
             },
-            _ => {}
+            FnCtxt::Assoc(_) => {}
         };
 
-        self.dcx().emit_err(errors::BadCVariadic { span: variadic_spans });
+        self.dcx().emit_err(errors::BadCVariadic { span: variadic_param.span });
     }
 
     fn check_item_named(&self, ident: Ident, kind: &str) {

compiler/rustc_ast_passes/src/errors.rs

@@ -322,7 +322,7 @@ pub(crate) struct ExternItemAscii {
 #[diag(ast_passes_bad_c_variadic)]
 pub(crate) struct BadCVariadic {
     #[primary_span]
-    pub span: Vec<Span>,
+    pub span: Span,
 }
 
 #[derive(Diagnostic)]
@@ -656,7 +656,7 @@ pub(crate) struct ConstAndCVariadic {
     #[label(ast_passes_const)]
     pub const_span: Span,
     #[label(ast_passes_variadic)]
-    pub variadic_spans: Vec<Span>,
+    pub variadic_span: Span,
 }
 
 #[derive(Diagnostic)]

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

@@ -47,10 +47,10 @@ 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
+  --> $DIR/issue-86053-1.rs:11:36
    |
 LL |     self , ... ,   self ,   self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
-   |            ^^^                     ^^^
+   |                                    ^^^
 
 error[E0412]: cannot find type `F` in this scope
   --> $DIR/issue-86053-1.rs:11:48

tests/ui/c-variadic/no-closure.rs

@@ -0,0 +1,17 @@
+#![feature(c_variadic)]
+#![crate_type = "lib"]
+
+// Check that `...` in closures is rejected.
+
+const F: extern "C" fn(...) = |_: ...| {};
+//~^ ERROR C-variadic type `...` may not be nested inside another type
+
+fn foo() {
+    let f = |...| {};
+    //~^ ERROR: `..` patterns are not allowed here
+    //~| ERROR: unexpected `...`
+
+    let f = |_: ...| {};
+    //~^ ERROR C-variadic type `...` may not be nested inside another type
+    f(1i64)
+}

tests/ui/c-variadic/no-closure.stderr

@@ -0,0 +1,35 @@
+error[E0743]: C-variadic type `...` may not be nested inside another type
+  --> $DIR/no-closure.rs:6:35
+   |
+LL | const F: extern "C" fn(...) = |_: ...| {};
+   |                                   ^^^
+
+error: unexpected `...`
+  --> $DIR/no-closure.rs:10:14
+   |
+LL |     let f = |...| {};
+   |              ^^^ not a valid pattern
+   |
+help: for a rest pattern, use `..` instead of `...`
+   |
+LL -     let f = |...| {};
+LL +     let f = |..| {};
+   |
+
+error[E0743]: C-variadic type `...` may not be nested inside another type
+  --> $DIR/no-closure.rs:14:17
+   |
+LL |     let f = |_: ...| {};
+   |                 ^^^
+
+error: `..` patterns are not allowed here
+  --> $DIR/no-closure.rs:10:14
+   |
+LL |     let f = |...| {};
+   |              ^^^
+   |
+   = note: only allowed in tuple, tuple struct, and slice patterns
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0743`.

tests/ui/c-variadic/valid.rs

@@ -0,0 +1,29 @@
+//@ run-pass
+#![feature(c_variadic)]
+
+// In rust (and C23 and above) `...` can be the only argument.
+unsafe extern "C" fn only_dot_dot_dot(mut ap: ...) -> i32 {
+    unsafe { ap.arg() }
+}
+
+unsafe extern "C-unwind" fn abi_c_unwind(mut ap: ...) -> i32 {
+    unsafe { ap.arg() }
+}
+
+#[allow(improper_ctypes_definitions)]
+unsafe extern "C" fn mix_int_float(mut ap: ...) -> (i64, f64, *const i32, f64) {
+    (ap.arg(), ap.arg(), ap.arg(), ap.arg())
+}
+
+fn main() {
+    unsafe {
+        assert_eq!(only_dot_dot_dot(32), 32);
+        assert_eq!(abi_c_unwind(32), 32);
+
+        // Passing more arguments than expected is allowed.
+        assert_eq!(only_dot_dot_dot(32, 1i64, core::ptr::null::<i32>(), 3.14f64), 32);
+
+        let ptr = &14i32 as *const i32;
+        assert_eq!(mix_int_float(12i64, 13.0f64, ptr, 15.0f64), (12, 13.0, ptr, 15.0));
+    }
+}

tests/ui/parser/variadic-ffi-semantic-restrictions.rs

@@ -16,8 +16,7 @@ extern "C" fn f2_2(...) {}
 //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
 
 extern "C" fn f2_3(..., x: isize) {}
-//~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
-//~| ERROR `...` must be the last argument of a C-variadic function
+//~^ ERROR `...` must be the last argument of a C-variadic function
 
 extern "C" fn f3_1(x: isize, ...) {}
 //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
@@ -26,8 +25,7 @@ extern "C" fn f3_2(...) {}
 //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
 
 extern "C" fn f3_3(..., x: isize) {}
-//~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
-//~| ERROR `...` must be the last argument of a C-variadic function
+//~^ ERROR `...` must be the last argument of a C-variadic function
 
 const unsafe extern "C" fn f4_1(x: isize, ...) {}
 //~^ ERROR functions cannot be both `const` and C-variadic
@@ -77,9 +75,7 @@ trait T {
     fn t_f4(...);
     //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
     fn t_f5(..., x: isize) {}
-    //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
-    //~| ERROR `...` must be the last argument of a C-variadic function
+    //~^ ERROR `...` must be the last argument of a C-variadic function
     fn t_f6(..., x: isize);
-    //~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
-    //~| ERROR `...` must be the last argument of a C-variadic function
+    //~^ ERROR `...` must be the last argument of a C-variadic function
 }