From fd48528d185f59f60e301bce1e01d670ff4bdb30 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Wed, 10 Sep 2025 00:17:22 +0200 Subject: c-variadic: allow inherent methods to be c-variadic --- compiler/rustc_ast_passes/src/ast_validation.rs | 54 +++++++++++++------------ 1 file changed, 28 insertions(+), 26 deletions(-) (limited to 'compiler') diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index a6ef89b553d..cdb3062c591 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -696,36 +696,38 @@ impl<'a> AstValidator<'a> { match fn_ctxt { FnCtxt::Foreign => return, - FnCtxt::Free => 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(), - }); + 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(), + }); + } } + 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(), - }); + 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); } } - 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 }; -- cgit 1.4.1-3-g733a5 From 01e83adc88653123fee444fdb930c16dd08da82d Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Wed, 10 Sep 2025 17:53:41 +0200 Subject: 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. --- compiler/rustc_ast_passes/messages.ftl | 2 - compiler/rustc_ast_passes/src/ast_validation.rs | 59 +++++++++---------- compiler/rustc_ast_passes/src/errors.rs | 7 --- compiler/rustc_middle/src/traits/mod.rs | 6 ++ .../src/traits/dyn_compatibility.rs | 3 + tests/ui/c-variadic/not-dyn-compatible.rs | 35 +++++++++++ tests/ui/c-variadic/not-dyn-compatible.stderr | 21 +++++++ tests/ui/c-variadic/trait-method.rs | 67 ++++++++++++++++------ tests/ui/c-variadic/trait-method.stderr | 14 ----- tests/ui/feature-gates/feature-gate-c_variadic.rs | 1 - .../feature-gates/feature-gate-c_variadic.stderr | 8 +-- .../parser/variadic-ffi-semantic-restrictions.rs | 8 +-- .../variadic-ffi-semantic-restrictions.stderr | 16 ++++-- 13 files changed, 158 insertions(+), 89 deletions(-) create mode 100644 tests/ui/c-variadic/not-dyn-compatible.rs create mode 100644 tests/ui/c-variadic/not-dyn-compatible.stderr delete mode 100644 tests/ui/c-variadic/trait-method.stderr (limited to 'compiler') 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 @@ -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] 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()` Generic, + /// e.g., `fn (mut ap: ...)` + CVariadic, + /// the method's receiver (`self` argument) can't be dispatched on UndispatchableReceiver(Option), } 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(...)`. 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::() } + } +} + +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 + --> $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::() } } } -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::() } + } + + unsafe extern "C" fn trait_method_ref(&self, mut ap: ...) -> i32 { + self.get() + unsafe { ap.arg::() } + } + + unsafe extern "C" fn trait_method_mut(&mut self, mut ap: ...) -> i32 { + self.get() + unsafe { ap.arg::() } + } + + unsafe extern "C" fn trait_fat_pointer(self: Box, mut ap: ...) -> i32 { + self.get() + unsafe { ap.arg::() } } } -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!(::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 = 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 -- cgit 1.4.1-3-g733a5