Skip to content

improve c-variadic errors #143546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 60 additions & 36 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2227,6 +2227,62 @@ pub struct FnSig {
pub span: Span,
}

impl FnSig {
/// Return a span encompassing the header, or where to insert it if empty.
pub fn header_span(&self) -> Span {
match self.header.ext {
Extern::Implicit(span) | Extern::Explicit(_, span) => {
return self.span.with_hi(span.hi());
}
Extern::None => {}
}

match self.header.safety {
Safety::Unsafe(span) | Safety::Safe(span) => return self.span.with_hi(span.hi()),
Safety::Default => {}
};

if let Some(coroutine_kind) = self.header.coroutine_kind {
return self.span.with_hi(coroutine_kind.span().hi());
}

if let Const::Yes(span) = self.header.constness {
return self.span.with_hi(span.hi());
}

self.span.shrink_to_lo()
}

/// The span of the header's safety, or where to insert it if empty.
pub fn safety_span(&self) -> Span {
match self.header.safety {
Safety::Unsafe(span) | Safety::Safe(span) => span,
Safety::Default => {
// Insert after the `coroutine_kind` if available.
if let Some(coroutine_kind) = self.header.coroutine_kind {
return coroutine_kind.span().shrink_to_hi();
}

// Insert after the `const` keyword if available.
if let Const::Yes(const_span) = self.header.constness {
return const_span.shrink_to_hi();
}

// Insert right at the front of the signature.
self.span.shrink_to_lo()
}
}
}

/// The span of the header's extern, or where to insert it if empty.
pub fn extern_span(&self) -> Span {
match self.header.ext {
Extern::Implicit(span) | Extern::Explicit(_, span) => span,
Extern::None => self.safety_span().shrink_to_hi(),
}
}
}

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
#[derive(Encodable, Decodable, HashStable_Generic)]
pub enum FloatTy {
Expand Down Expand Up @@ -3571,12 +3627,12 @@ impl Extern {
/// included in this struct (e.g., `async unsafe fn` or `const extern "C" fn`).
#[derive(Clone, Copy, Encodable, Decodable, Debug)]
pub struct FnHeader {
/// Whether this is `unsafe`, or has a default safety.
pub safety: Safety,
/// Whether this is `async`, `gen`, or nothing.
pub coroutine_kind: Option<CoroutineKind>,
/// The `const` keyword, if any
pub constness: Const,
/// Whether this is `async`, `gen`, or nothing.
pub coroutine_kind: Option<CoroutineKind>,
/// Whether this is `unsafe`, or has a default safety.
pub safety: Safety,
/// The `extern` keyword and corresponding ABI string, if any.
pub ext: Extern,
}
Expand All @@ -3590,38 +3646,6 @@ impl FnHeader {
|| matches!(constness, Const::Yes(_))
|| !matches!(ext, Extern::None)
}

/// Return a span encompassing the header, or none if all options are default.
pub fn span(&self) -> Option<Span> {
fn append(a: &mut Option<Span>, b: Span) {
*a = match a {
None => Some(b),
Some(x) => Some(x.to(b)),
}
}

let mut full_span = None;

match self.safety {
Safety::Unsafe(span) | Safety::Safe(span) => append(&mut full_span, span),
Safety::Default => {}
};

if let Some(coroutine_kind) = self.coroutine_kind {
append(&mut full_span, coroutine_kind.span());
}

if let Const::Yes(span) = self.constness {
append(&mut full_span, span);
}

match self.ext {
Extern::Implicit(span) | Extern::Explicit(_, span) => append(&mut full_span, span),
Extern::None => {}
}

full_span
}
}

impl Default for FnHeader {
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_ast_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,25 @@ 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 = only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg

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

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
Expand All @@ -73,6 +83,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
Expand Down
95 changes: 61 additions & 34 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,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 { " " };

Expand Down Expand Up @@ -633,46 +633,73 @@ 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();
let variadic_params: Vec<_> =
fk.decl().inputs.iter().filter(|arg| matches!(arg.ty.kind, TyKind::CVarArgs)).collect();

if variadic_spans.is_empty() {
// The parser already rejects `...` if it's not the final argument, but we still want to
// emit the errors below, so we only consider the final `...` here.
let Some(variadic_param) = variadic_params.last() else {
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")
};

// C-variadics are not yet implemented in const evaluation.
if let Const::Yes(const_span) = sig.header.constness {
self.dcx().emit_err(errors::ConstAndCVariadic {
span: const_span.to(variadic_param.span),
const_span,
variadic_span: variadic_param.span,
});
}

if let Some(header) = fk.header() {
if let Const::Yes(const_span) = header.constness {
let mut spans = variadic_spans.clone();
spans.push(const_span);
self.dcx().emit_err(errors::ConstAndCVariadic {
spans,
const_span,
variadic_spans: variadic_spans.clone(),
});
}
if let Some(coroutine_kind) = sig.header.coroutine_kind {
self.dcx().emit_err(errors::CoroutineAndCVariadic {
span: coroutine_kind.span().to(variadic_param.span),
coroutine_kind: coroutine_kind.as_str(),
coroutine_span: coroutine_kind.span(),
variadic_span: variadic_param.span,
});
}

match (fk.ctxt(), fk.header()) {
(Some(FnCtxt::Foreign), _) => return,
(Some(FnCtxt::Free), Some(header)) => match 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(_)) =>
{
return;
}
_ => {}
},
_ => {}
};
match fn_ctxt {
FnCtxt::Free => {
match sig.header.ext {
Extern::Implicit(_) => { /* defaults to "C" */ }
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 => {
self.dcx()
.emit_err(errors::CVariadicNoExtern { span: variadic_param.span });
}
};

self.dcx().emit_err(errors::BadCVariadic { span: variadic_spans });
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.
self.dcx()
.emit_err(errors::CVariadicAssociatedFunction { span: variadic_param.span });
}
FnCtxt::Foreign => {
// Whether the ABI supports C variable argument lists is checked later.
}
}
}

fn check_item_named(&self, ident: Ident, kind: &str) {
Expand Down
54 changes: 49 additions & 5 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,42 @@ pub(crate) struct ExternItemAscii {
}

#[derive(Diagnostic)]
#[diag(ast_passes_bad_c_variadic)]
pub(crate) struct BadCVariadic {
#[diag(ast_passes_c_variadic_associated_function)]
pub(crate) struct CVariadicAssociatedFunction {
#[primary_span]
pub span: Vec<Span>,
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_c_variadic_bad_extern)]
pub(crate) struct CVariadicBadExtern {
#[primary_span]
pub span: Span,
pub abi: Symbol,
#[label]
pub extern_span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_c_variadic_no_extern)]
pub(crate) struct CVariadicNoExtern {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_c_variadic_must_be_unsafe)]
pub(crate) struct CVariadicMustBeUnsafe {
#[primary_span]
pub span: Span,

#[suggestion(
ast_passes_suggestion,
applicability = "maybe-incorrect",
code = "unsafe ",
style = "verbose"
)]
pub unsafe_span: Span,
}

#[derive(Diagnostic)]
Expand Down Expand Up @@ -663,11 +695,23 @@ pub(crate) struct ConstAndCoroutine {
#[diag(ast_passes_const_and_c_variadic)]
pub(crate) struct ConstAndCVariadic {
#[primary_span]
pub spans: Vec<Span>,
pub span: Span,
#[label(ast_passes_const)]
pub const_span: Span,
#[label(ast_passes_variadic)]
pub variadic_spans: Vec<Span>,
pub variadic_span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_coroutine_and_c_variadic)]
pub(crate) struct CoroutineAndCVariadic {
#[primary_span]
pub span: Span,
pub coroutine_kind: &'static str,
#[label(ast_passes_const)]
pub coroutine_span: Span,
#[label(ast_passes_variadic)]
pub variadic_span: Span,
}

#[derive(Diagnostic)]
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/c-variadic/issue-86053-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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 only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
//~| ERROR `...` is not supported for non-extern functions
//~| ERROR: functions with a C variable argument list must be unsafe
//~| ERROR cannot find type `F` in this scope
}
19 changes: 15 additions & 4 deletions tests/ui/c-variadic/issue-86053-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,22 @@ error: `...` must be the last argument of a C-variadic function
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^

error: only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
--> $DIR/issue-86053-1.rs:11:12
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 ) {
| ^^^

error: functions with a C variable argument list must be unsafe
--> $DIR/issue-86053-1.rs:11:36
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^ ^^^
| ^^^
|
help: add the `unsafe` keyword to this definition
|
LL | unsafe fn ordering4 < 'a , 'b > ( a : , self , self , self ,
| ++++++

error[E0412]: cannot find type `F` in this scope
--> $DIR/issue-86053-1.rs:11:48
Expand All @@ -70,6 +81,6 @@ help: you might be missing a type parameter
LL | fn ordering4 < 'a , 'b, F > ( a : , self , self , self ,
| +++

error: aborting due to 10 previous errors
error: aborting due to 11 previous errors

For more information about this error, try `rustc --explain E0412`.
2 changes: 1 addition & 1 deletion tests/ui/c-variadic/issue-86053-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

trait H<T> {}

unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), ...) {}
unsafe extern "C" fn ordering4<'a, F: H<&'static &'a ()>>(_: (), _: ...) {}
//~^ ERROR: in type `&'static &'a ()`, reference has a longer lifetime than the data it references [E0491]

fn main() {}
Loading
Loading