c-variadic: allow trait methods to be c-variadic

but a C-variadic method makes a trait dyn-incompatible. That is because
methods from dyn traits, when cast to a function pointer, create a shim.
That shim can't really forward the c-variadic arguments.

13 files changed, 158 insertions, 89 deletions

diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl
index 8dcf3e3aa38..ba79fc63fad 100644
--- a/compiler/rustc_ast_passes/messages.ftl
+++ b/compiler/rustc_ast_passes/messages.ftl
@@ -64,8 +64,6 @@ ast_passes_body_in_extern = incorrect `{$kind}` inside `extern` block
 
 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
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
index cdb3062c591..c6dd625c228 100644
--- a/compiler/rustc_ast_passes/src/ast_validation.rs
+++ b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -696,43 +696,36 @@ impl<'a> AstValidator<'a> {
 
         match fn_ctxt {
             FnCtxt::Foreign => return,
-            FnCtxt::Free | FnCtxt::Assoc(AssocCtxt::Impl { of_trait: false }) => {
-                match sig.header.ext {
-                    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::Free | FnCtxt::Assoc(_) => match sig.header.ext {
+                Extern::Implicit(_) => {
+                    if !matches!(sig.header.safety, Safety::Unsafe(_)) {
+                        self.dcx().emit_err(errors::CVariadicMustBeUnsafe {
+                            span: variadic_param.span,
+                            unsafe_span: sig.safety_span(),
+                        });
                     }
-                    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(),
-                            });
-                        }
-
-                        if !matches!(sig.header.safety, Safety::Unsafe(_)) {
-                            self.dcx().emit_err(errors::CVariadicMustBeUnsafe {
-                                span: variadic_param.span,
-                                unsafe_span: sig.safety_span(),
-                            });
-                        }
+                }
+                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 => {
-                        let err = errors::CVariadicNoExtern { span: variadic_param.span };
-                        self.dcx().emit_err(err);
+
+                    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.
-                let err = errors::CVariadicAssociatedFunction { span: variadic_param.span };
-                self.dcx().emit_err(err);
-            }
+                Extern::None => {
+                    let err = errors::CVariadicNoExtern { span: variadic_param.span };
+                    self.dcx().emit_err(err);
+                }
+            },
         }
     }
 
diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs
index ae805042c54..b1b1ad60c24 100644
--- a/compiler/rustc_ast_passes/src/errors.rs
+++ b/compiler/rustc_ast_passes/src/errors.rs
@@ -319,13 +319,6 @@ pub(crate) struct ExternItemAscii {
 }
 
 #[derive(Diagnostic)]
-#[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 {
diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs
index ab8a3142953..0c7bddf60d9 100644
--- a/compiler/rustc_middle/src/traits/mod.rs
+++ b/compiler/rustc_middle/src/traits/mod.rs
@@ -823,6 +823,9 @@ impl DynCompatibilityViolation {
             DynCompatibilityViolation::Method(name, MethodViolationCode::AsyncFn, _) => {
                 format!("method `{name}` is `async`").into()
             }
+            DynCompatibilityViolation::Method(name, MethodViolationCode::CVariadic, _) => {
+                format!("method `{name}` is C-variadic").into()
+            }
             DynCompatibilityViolation::Method(
                 name,
                 MethodViolationCode::WhereClauseReferencesSelf,
@@ -977,6 +980,9 @@ pub enum MethodViolationCode {
     /// e.g., `fn foo<A>()`
     Generic,
 
+    /// e.g., `fn (mut ap: ...)`
+    CVariadic,
+
     /// the method's receiver (`self` argument) can't be dispatched on
     UndispatchableReceiver(Option<Span>),
 }
diff --git a/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs b/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
index bcd11d6918d..3260dd712b9 100644
--- a/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
+++ b/compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs
@@ -426,6 +426,9 @@ fn virtual_call_violations_for_method<'tcx>(
     if let Some(code) = contains_illegal_impl_trait_in_trait(tcx, method.def_id, sig.output()) {
         errors.push(code);
     }
+    if sig.skip_binder().c_variadic {
+        errors.push(MethodViolationCode::CVariadic);
+    }
 
     // We can't monomorphize things like `fn foo<A>(...)`.
     let own_counts = tcx.generics_of(method.def_id).own_counts();
diff --git a/tests/ui/c-variadic/not-dyn-compatible.rs b/tests/ui/c-variadic/not-dyn-compatible.rs
new file mode 100644
index 00000000000..b40a13e5847
--- /dev/null
+++ b/tests/ui/c-variadic/not-dyn-compatible.rs
@@ -0,0 +1,35 @@
+// Traits where a method is c-variadic are not dyn compatible.
+//
+// Creating a function pointer from a method on an `&dyn T` value creates a ReifyShim.
+// This shim cannot reliably forward C-variadic arguments. Thus the trait as a whole
+// is dyn-incompatible to prevent invalid shims from being created.
+#![feature(c_variadic)]
+
+#[repr(transparent)]
+struct Struct(u64);
+
+trait Trait {
+    fn get(&self) -> u64;
+
+    unsafe extern "C" fn dyn_method_ref(&self, mut ap: ...) -> u64 {
+        self.get() + unsafe { ap.arg::<u64>() }
+    }
+}
+
+impl Trait for Struct {
+    fn get(&self) -> u64 {
+        self.0
+    }
+}
+
+fn main() {
+    unsafe {
+        let dyn_object: &dyn Trait = &Struct(64);
+        //~^ ERROR the trait `Trait` is not dyn compatible
+        assert_eq!(dyn_object.dyn_method_ref(100), 164);
+        assert_eq!(
+            (Trait::dyn_method_ref as unsafe extern "C" fn(_, ...) -> u64)(dyn_object, 100),
+            164
+        );
+    }
+}
diff --git a/tests/ui/c-variadic/not-dyn-compatible.stderr b/tests/ui/c-variadic/not-dyn-compatible.stderr
new file mode 100644
index 00000000000..76630600c51
--- /dev/null
+++ b/tests/ui/c-variadic/not-dyn-compatible.stderr
@@ -0,0 +1,21 @@
+error[E0038]: the trait `Trait` is not dyn compatible
+  --> $DIR/not-dyn-compatible.rs:27:30
+   |
+LL |         let dyn_object: &dyn Trait = &Struct(64);
+   |                              ^^^^^ `Trait` is not dyn compatible
+   |
+note: for a trait to be dyn compatible it needs to allow building a vtable
+      for more information, visit <https://doc.rust-lang.org/reference/items/traits.html#dyn-compatibility>
+  --> $DIR/not-dyn-compatible.rs:14:26
+   |
+LL | trait Trait {
+   |       ----- this trait is not dyn compatible...
+...
+LL |     unsafe extern "C" fn dyn_method_ref(&self, mut ap: ...) -> u64 {
+   |                          ^^^^^^^^^^^^^^ ...because method `dyn_method_ref` is C-variadic
+   = help: consider moving `dyn_method_ref` to another trait
+   = help: only type `Struct` implements `Trait`; consider using it directly instead.
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0038`.
diff --git a/tests/ui/c-variadic/trait-method.rs b/tests/ui/c-variadic/trait-method.rs
index 4c468c1907b..97da0706a3a 100644
--- a/tests/ui/c-variadic/trait-method.rs
+++ b/tests/ui/c-variadic/trait-method.rs
@@ -1,40 +1,73 @@
-// For now C-variadic arguments in trait methods are rejected, though we aim to lift this
-// restriction in the future. In particular we need to think about the interaction with
-// `dyn Trait` and the `ReifyShim`s that it may generate for methods.
+//@ run-pass
 #![feature(c_variadic)]
-#![crate_type = "lib"]
-struct S;
 
-impl S {
+#[repr(transparent)]
+struct Struct(i32);
+
+impl Struct {
     unsafe extern "C" fn associated_function(mut ap: ...) -> i32 {
         unsafe { ap.arg() }
     }
 
     unsafe extern "C" fn method(&self, mut ap: ...) -> i32 {
-        unsafe { ap.arg() }
+        self.0 + unsafe { ap.arg::<i32>() }
     }
 }
 
-trait T {
+trait Trait: Sized {
+    fn get(&self) -> i32;
+
     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() }
+    unsafe extern "C" fn trait_method_owned(self, mut ap: ...) -> i32 {
+        self.get() + unsafe { ap.arg::<i32>() }
+    }
+
+    unsafe extern "C" fn trait_method_ref(&self, mut ap: ...) -> i32 {
+        self.get() + unsafe { ap.arg::<i32>() }
+    }
+
+    unsafe extern "C" fn trait_method_mut(&mut self, mut ap: ...) -> i32 {
+        self.get() + unsafe { ap.arg::<i32>() }
+    }
+
+    unsafe extern "C" fn trait_fat_pointer(self: Box<Self>, mut ap: ...) -> i32 {
+        self.get() + unsafe { ap.arg::<i32>() }
     }
 }
 
-impl T for S {}
+impl Trait for Struct {
+    fn get(&self) -> i32 {
+        self.0
+    }
+}
 
 fn main() {
     unsafe {
-        assert_eq!(S::associated_function(32), 32);
-        assert_eq!(S.method(32), 32);
+        assert_eq!(Struct::associated_function(32), 32);
+        assert_eq!(Struct(100).method(32), 132);
+
+        assert_eq!(Struct::trait_associated_function(32), 32);
+        assert_eq!(Struct(100).trait_method_owned(32), 132);
+        assert_eq!(Struct(100).trait_method_ref(32), 132);
+        assert_eq!(Struct(100).trait_method_mut(32), 132);
+        assert_eq!(Struct::trait_fat_pointer(Box::new(Struct(100)), 32), 132);
+
+        assert_eq!(<Struct as Trait>::trait_associated_function(32), 32);
+        assert_eq!(Trait::trait_method_owned(Struct(100), 32), 132);
+        assert_eq!(Trait::trait_method_ref(&Struct(100), 32), 132);
+        assert_eq!(Trait::trait_method_mut(&mut Struct(100), 32), 132);
+        assert_eq!(Trait::trait_fat_pointer(Box::new(Struct(100)), 32), 132);
+
+        type Associated = unsafe extern "C" fn(...) -> i32;
+        type Method<T> = unsafe extern "C" fn(T, ...) -> i32;
 
-        assert_eq!(S::trait_associated_function(32), 32);
-        assert_eq!(S.trait_method(32), 32);
+        assert_eq!((Struct::trait_associated_function as Associated)(32), 32);
+        assert_eq!((Struct::trait_method_owned as Method<_>)(Struct(100), 32), 132);
+        assert_eq!((Struct::trait_method_ref as Method<_>)(&Struct(100), 32), 132);
+        assert_eq!((Struct::trait_method_mut as Method<_>)(&mut Struct(100), 32), 132);
+        assert_eq!((Struct::trait_fat_pointer as Method<_>)(Box::new(Struct(100)), 32), 132);
     }
 }
diff --git a/tests/ui/c-variadic/trait-method.stderr b/tests/ui/c-variadic/trait-method.stderr
deleted file mode 100644
index 35f6e24ef71..00000000000
--- a/tests/ui/c-variadic/trait-method.stderr
+++ /dev/null
@@ -1,14 +0,0 @@
-error: associated functions cannot have a C variable argument list
-  --> $DIR/trait-method.rs:19:52
-   |
-LL |     unsafe extern "C" fn trait_associated_function(mut ap: ...) -> i32 {
-   |                                                    ^^^^^^^^^^^
-
-error: associated functions cannot have a C variable argument list
-  --> $DIR/trait-method.rs:24:46
-   |
-LL |     unsafe extern "C" fn trait_method(&self, mut ap: ...) -> i32 {
-   |                                              ^^^^^^^^^^^
-
-error: aborting due to 2 previous errors
-
diff --git a/tests/ui/feature-gates/feature-gate-c_variadic.rs b/tests/ui/feature-gates/feature-gate-c_variadic.rs
index 88d91dbd081..649816b48d7 100644
--- a/tests/ui/feature-gates/feature-gate-c_variadic.rs
+++ b/tests/ui/feature-gates/feature-gate-c_variadic.rs
@@ -6,5 +6,4 @@ pub unsafe extern "C" fn test(_: i32, ap: ...) {}
 trait Trait {
     unsafe extern "C" fn trait_test(_: i32, ap: ...) {}
     //~^ ERROR C-variadic functions are unstable
-    //~| ERROR associated functions cannot have a C variable argument list
 }
diff --git a/tests/ui/feature-gates/feature-gate-c_variadic.stderr b/tests/ui/feature-gates/feature-gate-c_variadic.stderr
index 808aa20948d..ae880093b98 100644
--- a/tests/ui/feature-gates/feature-gate-c_variadic.stderr
+++ b/tests/ui/feature-gates/feature-gate-c_variadic.stderr
@@ -1,9 +1,3 @@
-error: associated functions cannot have a C variable argument list
-  --> $DIR/feature-gate-c_variadic.rs:7:45
-   |
-LL |     unsafe extern "C" fn trait_test(_: i32, ap: ...) {}
-   |                                             ^^^^^^^
-
 error[E0658]: C-variadic functions are unstable
   --> $DIR/feature-gate-c_variadic.rs:3:1
    |
@@ -24,6 +18,6 @@ LL |     unsafe extern "C" fn trait_test(_: i32, ap: ...) {}
    = help: add `#![feature(c_variadic)]` to the crate attributes to enable
    = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
 
-error: aborting due to 3 previous errors
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0658`.
diff --git a/tests/ui/parser/variadic-ffi-semantic-restrictions.rs b/tests/ui/parser/variadic-ffi-semantic-restrictions.rs
index 566c4d30245..415472176d9 100644
--- a/tests/ui/parser/variadic-ffi-semantic-restrictions.rs
+++ b/tests/ui/parser/variadic-ffi-semantic-restrictions.rs
@@ -70,13 +70,13 @@ impl X {
 
 trait T {
     fn t_f1(x: isize, ...) {}
-    //~^ ERROR associated functions cannot have a C variable argument list
+    //~^ ERROR `...` is not supported for non-extern functions
     fn t_f2(x: isize, ...);
-    //~^ ERROR associated functions cannot have a C variable argument list
+    //~^ ERROR `...` is not supported for non-extern functions
     fn t_f3(...) {}
-    //~^ ERROR associated functions cannot have a C variable argument list
+    //~^ ERROR `...` is not supported for non-extern functions
     fn t_f4(...);
-    //~^ ERROR associated functions cannot have a C variable argument list
+    //~^ ERROR `...` is not supported for non-extern functions
     fn t_f5(..., x: isize) {}
     //~^ ERROR `...` must be the last argument of a C-variadic function
     fn t_f6(..., x: isize);
diff --git a/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr b/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr
index c18d857d14e..da9c9b0f760 100644
--- a/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr
+++ b/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr
@@ -192,29 +192,37 @@ LL |     const fn i_f5(x: isize, ...) {}
    |
    = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
-error: associated functions cannot have a C variable argument list
+error: `...` is not supported for non-extern functions
   --> $DIR/variadic-ffi-semantic-restrictions.rs:72:23
    |
 LL |     fn t_f1(x: isize, ...) {}
    |                       ^^^
+   |
+   = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
-error: associated functions cannot have a C variable argument list
+error: `...` is not supported for non-extern functions
   --> $DIR/variadic-ffi-semantic-restrictions.rs:74:23
    |
 LL |     fn t_f2(x: isize, ...);
    |                       ^^^
+   |
+   = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
-error: associated functions cannot have a C variable argument list
+error: `...` is not supported for non-extern functions
   --> $DIR/variadic-ffi-semantic-restrictions.rs:76:13
    |
 LL |     fn t_f3(...) {}
    |             ^^^
+   |
+   = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
-error: associated functions cannot have a C variable argument list
+error: `...` is not supported for non-extern functions
   --> $DIR/variadic-ffi-semantic-restrictions.rs:78:13
    |
 LL |     fn t_f4(...);
    |             ^^^
+   |
+   = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
 error: `...` must be the last argument of a C-variadic function
   --> $DIR/variadic-ffi-semantic-restrictions.rs:80:13