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.

Author: folkertdev

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

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);
+                }
+            },
         }
     }
 

compiler/rustc_ast_passes/src/errors.rs

@@ -318,13 +318,6 @@ pub(crate) struct ExternItemAscii {
     pub block: Span,
 }
 
-#[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]

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>),
 }

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();

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
+        );
+    }
+}

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`.

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);
     }
 }

tests/ui/c-variadic/trait-method.stderr

@@ -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
 }