Rollup merge of #146342 - folkertdev:c-variadic-errors-take-3, r=workingjubilee

Improve C-variadic error messages: part 2
tracking issue: #44930
a reimplementation of #143546 that builds on #146165.
This PR
- disallows coroutines (e.g. `async fn`) from having a `...` argument
- disallows associated functions (both in traits and standard impl blocks) from having a `...` argument
- splits up a generic "ill-formed C-variadic function" into specific errors about using an incorrect ABI, not specifying an ABI, or missing the unsafe keyword
C-variadic coroutines probably don't make sense? C-variadic functions are for FFI purposes, combining that with async functions seems weird.
For associated functions, we're just cutting scope. It's probably fine, but it's probably better to explicitly allow it. So for now, at least give a more targeted error message.
Made to be reviewed commit-by-commit.
cc `@workingjubilee`
r? compiler

Author: matthiaskrgr

compiler/rustc_ast/src/ast.rs

@@ -2284,6 +2284,54 @@ pub struct FnSig {
 pubspan:Span,
 }
 
+implFnSig{
+/// Return a span encompassing the header, or where to insert it if empty.
+pubfnheader_span(&self) -> Span{
+matchself.header.ext{
+Extern::Implicit(span) | Extern::Explicit(_, span) => {
+returnself.span.with_hi(span.hi());
+}
+Extern::None => {}
+}
+
+matchself.header.safety{
+Safety::Unsafe(span) | Safety::Safe(span) => returnself.span.with_hi(span.hi()),
+Safety::Default => {}
+};
+
+ifletSome(coroutine_kind) = self.header.coroutine_kind{
+returnself.span.with_hi(coroutine_kind.span().hi());
+}
+
+ifletConst::Yes(span) = self.header.constness{
+returnself.span.with_hi(span.hi());
+}
+
+self.span.shrink_to_lo()
+}
+
+/// The span of the header's safety, or where to insert it if empty.
+pubfnsafety_span(&self) -> Span{
+matchself.header.safety{
+Safety::Unsafe(span) | Safety::Safe(span) => span,
+Safety::Default => {
+// Insert after the `coroutine_kind` if available.
+ifletSome(extern_span) = self.header.ext.span(){
+return extern_span.shrink_to_lo();
+}
+
+// Insert right at the front of the signature.
+self.header_span().shrink_to_hi()
+}
+}
+}
+
+/// The span of the header's extern, or where to insert it if empty.
+pubfnextern_span(&self) -> Span{
+self.header.ext.span().unwrap_or(self.safety_span().shrink_to_hi())
+}
+}
+
 /// A constraint on an associated item.
 ///
 /// ### Examples
@@ -3526,6 +3574,13 @@ impl Extern {
 None => Extern::Implicit(span),
 }
 }
+
+pubfnspan(self) -> Option<Span>{
+matchself{
+Extern::None => None,
+Extern::Implicit(span) | Extern::Explicit(_, span) => Some(span),
+}
+}
 }
 
 /// A function header.
@@ -3534,12 +3589,12 @@ impl Extern {
 /// included in this struct (e.g., `async unsafe fn` or `const extern "C" fn`).
 #[derive(Clone,Copy,Encodable,Decodable,Debug,Walkable)]
 pubstructFnHeader{
-/// Whether this is `unsafe`, or has a default safety.
-pubsafety:Safety,
-/// Whether this is `async`, `gen`, or nothing.
-pubcoroutine_kind:Option<CoroutineKind>,
 /// The `const` keyword, if any
 pubconstness:Const,
+/// Whether this is `async`, `gen`, or nothing.
+pubcoroutine_kind:Option<CoroutineKind>,
+/// Whether this is `unsafe`, or has a default safety.
+pubsafety:Safety,
 /// The `extern` keyword and corresponding ABI string, if any.
 pubext:Extern,
 }
@@ -3553,38 +3608,6 @@ impl FnHeader {
  || matches!(constness,Const::Yes(_))
  || !matches!(ext,Extern::None)
 }
-
-/// Return a span encompassing the header, or none if all options are default.
-pubfnspan(&self) -> Option<Span>{
-fnappend(a:&mutOption<Span>,b:Span){
-*a = match a {
-None => Some(b),
-Some(x) => Some(x.to(b)),
-}
-}
-
-letmut full_span = None;
-
-matchself.safety{
-Safety::Unsafe(span) | Safety::Safe(span) => append(&mut full_span, span),
-Safety::Default => {}
-};
-
-ifletSome(coroutine_kind) = self.coroutine_kind{
-append(&mut full_span, coroutine_kind.span());
-}
-
-ifletConst::Yes(span) = self.constness{
-append(&mut full_span, span);
-}
-
-matchself.ext{
-Extern::Implicit(span) | Extern::Explicit(_, span) => append(&mut full_span, span),
-Extern::None => {}
-}
-
- full_span
-}
 }
 
 implDefaultforFnHeader{

compiler/rustc_ast_passes/messages.ftl

@@ -57,15 +57,26 @@ 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
+.help = only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
+
 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 +95,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

@@ -492,7 +492,7 @@ impl<'a> AstValidator<'a> {
 }
 
 if !spans.is_empty(){
-let header_span = sig.header.span().unwrap_or(sig.span.shrink_to_lo());
+let header_span = sig.header_span();
 let suggestion_span = header_span.shrink_to_hi().to(sig.decl.output.span());
 let padding = if header_span.is_empty(){""}else{" "};
 
@@ -685,22 +685,53 @@ impl<'a> AstValidator<'a> {
 });
 }
 
+ifletSome(coroutine_kind) = sig.header.coroutine_kind{
+self.dcx().emit_err(errors::CoroutineAndCVariadic{
+spans:vec![coroutine_kind.span(), variadic_param.span],
+coroutine_kind: coroutine_kind.as_str(),
+coroutine_span: coroutine_kind.span(),
+variadic_span: variadic_param.span,
+});
+}
+
 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(_)
-ifmatches!(sig.header.safety,Safety::Unsafe(_)) =>
-{
-return;
+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::Assoc(_) => {}
-};
+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(),
+});
+}
 
-self.dcx().emit_err(errors::BadCVariadic{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(),
+});
+}
+}
+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};
+self.dcx().emit_err(err);
+}
+}
 }
 
 fncheck_item_named(&self,ident:Ident,kind:&str){

compiler/rustc_ast_passes/src/errors.rs

@@ -319,12 +319,46 @@ pub(crate) struct ExternItemAscii {
 }
 
 #[derive(Diagnostic)]
-#[diag(ast_passes_bad_c_variadic)]
-pub(crate)structBadCVariadic{
+#[diag(ast_passes_c_variadic_associated_function)]
+pub(crate)structCVariadicAssociatedFunction{
 #[primary_span]
 pubspan:Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_no_extern)]
+#[help]
+pub(crate)structCVariadicNoExtern{
+#[primary_span]
+pubspan:Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_must_be_unsafe)]
+pub(crate)structCVariadicMustBeUnsafe{
+#[primary_span]
+pubspan:Span,
+
+#[suggestion(
+ ast_passes_suggestion,
+ applicability = "maybe-incorrect",
+ code = "unsafe ",
+ style = "verbose"
+)]
+pubunsafe_span:Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_c_variadic_bad_extern)]
+#[help]
+pub(crate)structCVariadicBadExtern{
+#[primary_span]
+pubspan:Span,
+pubabi:Symbol,
+#[label]
+pubextern_span:Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_item_underscore)]
 pub(crate)structItemUnderscore<'a>{
@@ -659,6 +693,18 @@ pub(crate) struct ConstAndCVariadic {
 pubvariadic_span:Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_coroutine_and_c_variadic)]
+pub(crate)structCoroutineAndCVariadic{
+#[primary_span]
+pubspans:Vec<Span>,
+pubcoroutine_kind:&'staticstr,
+#[label(ast_passes_const)]
+pubcoroutine_span:Span,
+#[label(ast_passes_variadic)]
+pubvariadic_span:Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_pattern_in_foreign, code = E0130)]
 // FIXME: deduplicate with rustc_lint (`BuiltinLintDiag::PatternsInFnsWithoutBody`)

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

@@ -13,6 +13,6 @@ 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 cannot find type `F` in this scope
 }

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

@@ -46,11 +46,13 @@ 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
+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 ) {
  | ^^^
+ |
+ = help: only `extern "C"` and `extern "C-unwind"` functions may have a C variable argument list
 
 error[E0412]: cannot find type `F` in this scope
  --> $DIR/issue-86053-1.rs:11:48

tests/ui/c-variadic/not-async.rs

@@ -0,0 +1,7 @@
+//@ edition: 2021
+#![feature(c_variadic)]
+#![crate_type = "lib"]
+
+asyncunsafeextern"C"fncannot_be_async(x:isize, ...){}
+//~^ ERROR functions cannot be both `async` and C-variadic
+//~| ERROR hidden type for `impl Future<Output = ()>` captures lifetime that does not appear in bounds

tests/ui/c-variadic/not-async.stderr

@@ -0,0 +1,19 @@
+error: functions cannot be both `async` and C-variadic
+ --> $DIR/not-async.rs:5:1
+ |
+LL | async unsafe extern "C" fn cannot_be_async(x: isize, ...) {}
+ | ^^^^^ `async` because of this ^^^ C-variadic because of this
+
+error[E0700]: hidden type for `impl Future<Output = ()>` captures lifetime that does not appear in bounds
+ --> $DIR/not-async.rs:5:59
+ |
+LL | async unsafe extern "C" fn cannot_be_async(x: isize, ...) {}
+ | --------------------------------------------------------- ^^
+ | |
+ | opaque type defined here
+ |
+ = note: hidden type `{async fn body of cannot_be_async()}` captures lifetime `'_`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0700`.

tests/ui/cmse-nonsecure/cmse-nonsecure-entry/generics.rs

@@ -53,24 +53,20 @@ extern "cmse-nonsecure-entry" fn trait_object(x: &dyn Trait) -> &dyn Trait {
  x
 }
 
-extern"cmse-nonsecure-entry"fnstatic_trait_object(
-x:&'staticdynTrait,
-) -> &'staticdynTrait{
+extern"cmse-nonsecure-entry"fnstatic_trait_object(x:&'staticdynTrait) -> &'staticdynTrait{
 //~^ ERROR return value of `"cmse-nonsecure-entry"` function too large to pass via registers [E0798]
  x
 }
 
 #[repr(transparent)]
 structWrapperTransparent<'a>(&'adynTrait);
 
-extern"cmse-nonsecure-entry"fnwrapped_trait_object(
-x:WrapperTransparent,
-) -> WrapperTransparent{
+extern"cmse-nonsecure-entry"fnwrapped_trait_object(x:WrapperTransparent) -> WrapperTransparent{
 //~^ ERROR return value of `"cmse-nonsecure-entry"` function too large to pass via registers [E0798]
  x
 }
 
-extern"cmse-nonsecure-entry"fnc_variadic(_:u32, _: ...){
-//~^ ERROR defining functions with C-variadic arguments is only allowed for free functions with the "C" or "C-unwind" calling convention
+unsafeextern"cmse-nonsecure-entry"fnc_variadic(_:u32, _: ...){
+//~^ ERROR `...` is not supported for `extern "cmse-nonsecure-entry"` functions
 //~| ERROR requires `va_list` lang_item
 }