Skip to content

Commit

Permalink
Rollup merge of rust-lang#117370 - nicholasbishop:bishop-better-c-var…
Browse files Browse the repository at this point in the history 
…iadic-errors, r=oli-obk

C-variadic error improvements

A couple improvements for c-variadic errors:

1. Fix the bad-c-variadic error being emitted multiple times. If a function incorrectly contains multiple `...` args, and is also not foreign or `unsafe extern "C"`, only emit the latter error once rather than once per `...`.

2. Explicitly reject `const` C-variadic functions. Trying to use C-variadics in a const function would previously fail with an error like "destructor of `VaListImpl<'_>` cannot be evaluated at compile-time". Add an explicit check for const C-variadics to provide a clearer error: "functions cannot be both `const` and C-variadic". This also addresses one of the concerns in rust-lang#44930: "Ensure that even when this gets stabilized for regular functions, it is still rejected on const fn."
  • Loading branch information
@GuillaumeGomez
GuillaumeGomez committed Oct 30, 2023
2 parents 5ac999f + f91b5ce commit 784f04b
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 49 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_ast_passes/messages.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ ast_passes_const_and_async = functions cannot be both `const` and `async`
.async = `async` because of this
.label = {""}
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
ast_passes_const_without_body =
free constant item without body
.suggestion = provide a definition for the constant
Expand Down
37 changes: 30 additions & 7 deletions compiler/rustc_ast_passes/src/ast_validation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,36 @@ impl<'a> AstValidator<'a> {
}
}

/// Reject C-variadic type unless the function is foreign,
/// or free and `unsafe extern "C"` semantically.
/// Reject invalid C-variadic types.
///
/// C-variadics must be:
/// - 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();

if variadic_spans.is_empty() {
return;
}

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.err_handler().emit_err(errors::ConstAndCVariadic {
spans,
const_span,
variadic_spans: variadic_spans.clone(),
});
}
}

match (fk.ctxt(), fk.header()) {
(Some(FnCtxt::Foreign), _) => return,
(Some(FnCtxt::Free), Some(header)) => match header.ext {
Expand All @@ -499,11 +526,7 @@ impl<'a> AstValidator<'a> {
_ => {}
};

for Param { ty, span, .. } in &fk.decl().inputs {
if let TyKind::CVarArgs = ty.kind {
self.err_handler().emit_err(errors::BadCVariadic { span: *span });
}
}
self.err_handler().emit_err(errors::BadCVariadic { span: variadic_spans });
}

fn check_item_named(&self, ident: Ident, kind: &str) {
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_ast_passes/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ pub struct ExternItemAscii {
#[diag(ast_passes_bad_c_variadic)]
pub struct BadCVariadic {
#[primary_span]
pub span: Span,
pub span: Vec<Span>,
}

#[derive(Diagnostic)]
Expand Down Expand Up @@ -583,6 +583,17 @@ pub struct ConstAndAsync {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_const_and_c_variadic)]
pub struct ConstAndCVariadic {
#[primary_span]
pub spans: Vec<Span>,
#[label(ast_passes_const)]
pub const_span: Span,
#[label(ast_passes_variadic)]
pub variadic_spans: Vec<Span>,
}

#[derive(Diagnostic)]
#[diag(ast_passes_pattern_in_foreign, code = "E0130")]
pub struct PatternInForeign {
Expand Down
10 changes: 2 additions & 8 deletions tests/ui/c-variadic/issue-86053-1.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,7 @@ error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/issue-86053-1.rs:11:12
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/issue-86053-1.rs:11:36
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^
| ^^^ ^^^

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

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

For more information about this error, try `rustc --explain E0412`.
17 changes: 15 additions & 2 deletions tests/ui/parser/variadic-ffi-semantic-restrictions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ extern "C" fn f3_3(..., x: isize) {}
//~^ ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR `...` must be the last argument of a C-variadic function

const unsafe extern "C" fn f4_1(x: isize, ...) {}
//~^ ERROR functions cannot be both `const` and C-variadic

const extern "C" fn f4_2(x: isize, ...) {}
//~^ ERROR functions cannot be both `const` and C-variadic
//~| ERROR only foreign or `unsafe extern "C"` functions may be C-variadic

const extern "C" fn f4_3(..., x: isize, ...) {}
//~^ ERROR functions cannot be both `const` and C-variadic
//~| ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR `...` must be the last argument of a C-variadic function

extern "C" {
fn e_f1(...);
//~^ ERROR C-variadic function must be declared with at least one named argument
Expand All @@ -49,12 +61,13 @@ impl X {
//~| ERROR C-variadic function must be declared with at least one named argument
fn i_f3(..., x: isize, ...) {}
//~^ ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR `...` must be the last argument of a C-variadic function
fn i_f4(..., x: isize, ...) {}
//~^ ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR `...` must be the last argument of a C-variadic function
const fn i_f5(x: isize, ...) {}
//~^ ERROR only foreign or `unsafe extern "C"` functions may be C-variadic
//~| ERROR functions cannot be both `const` and C-variadic
}

trait T {
Expand Down
103 changes: 72 additions & 31 deletions tests/ui/parser/variadic-ffi-semantic-restrictions.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,131 +76,172 @@ error: only foreign or `unsafe extern "C"` functions may be C-variadic
LL | extern "C" fn f3_3(..., x: isize) {}
| ^^^

error: functions cannot be both `const` and C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:35:1
|
LL | const unsafe extern "C" fn f4_1(x: isize, ...) {}
| ^^^^^ `const` because of this ^^^ C-variadic because of this

error: functions cannot be both `const` and C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:38:1
|
LL | const extern "C" fn f4_2(x: isize, ...) {}
| ^^^^^ `const` because of this ^^^ C-variadic because of this

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:38:36
|
LL | const extern "C" fn f4_2(x: isize, ...) {}
| ^^^

error: `...` must be the last argument of a C-variadic function
--> $DIR/variadic-ffi-semantic-restrictions.rs:42:26
|
LL | const extern "C" fn f4_3(..., x: isize, ...) {}
| ^^^

error: functions cannot be both `const` and C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:42:1
|
LL | const extern "C" fn f4_3(..., x: isize, ...) {}
| ^^^^^ ^^^ ^^^ C-variadic because of this
| | |
| | C-variadic because of this
| `const` because of this

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:42:26
|
LL | const extern "C" fn f4_3(..., x: isize, ...) {}
| ^^^ ^^^

error: C-variadic function must be declared with at least one named argument
--> $DIR/variadic-ffi-semantic-restrictions.rs:36:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:48:13
|
LL | fn e_f1(...);
| ^^^

error: `...` must be the last argument of a C-variadic function
--> $DIR/variadic-ffi-semantic-restrictions.rs:38:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:50:13
|
LL | fn e_f2(..., x: isize);
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:45:23
--> $DIR/variadic-ffi-semantic-restrictions.rs:57:23
|
LL | fn i_f1(x: isize, ...) {}
| ^^^

error: C-variadic function must be declared with at least one named argument
--> $DIR/variadic-ffi-semantic-restrictions.rs:47:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:59:13
|
LL | fn i_f2(...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:47:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:59:13
|
LL | fn i_f2(...) {}
| ^^^

error: `...` must be the last argument of a C-variadic function
--> $DIR/variadic-ffi-semantic-restrictions.rs:50:13
|
LL | fn i_f3(..., x: isize, ...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:50:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:62:13
|
LL | fn i_f3(..., x: isize, ...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:50:28
--> $DIR/variadic-ffi-semantic-restrictions.rs:62:13
|
LL | fn i_f3(..., x: isize, ...) {}
| ^^^
| ^^^ ^^^

error: `...` must be the last argument of a C-variadic function
--> $DIR/variadic-ffi-semantic-restrictions.rs:54:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:65:13
|
LL | fn i_f4(..., x: isize, ...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:54:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:65:13
|
LL | fn i_f4(..., x: isize, ...) {}
| ^^^
| ^^^ ^^^

error: functions cannot be both `const` and C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:68:5
|
LL | const fn i_f5(x: isize, ...) {}
| ^^^^^ ^^^ C-variadic because of this
| |
| `const` because of this

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:54:28
--> $DIR/variadic-ffi-semantic-restrictions.rs:68:29
|
LL | fn i_f4(..., x: isize, ...) {}
| ^^^
LL | const fn i_f5(x: isize, ...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:61:23
--> $DIR/variadic-ffi-semantic-restrictions.rs:74:23
|
LL | fn t_f1(x: isize, ...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:63:23
--> $DIR/variadic-ffi-semantic-restrictions.rs:76:23
|
LL | fn t_f2(x: isize, ...);
| ^^^

error: C-variadic function must be declared with at least one named argument
--> $DIR/variadic-ffi-semantic-restrictions.rs:65:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:78:13
|
LL | fn t_f3(...) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:65:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:78:13
|
LL | fn t_f3(...) {}
| ^^^

error: C-variadic function must be declared with at least one named argument
--> $DIR/variadic-ffi-semantic-restrictions.rs:68:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:81:13
|
LL | fn t_f4(...);
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:68:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:81:13
|
LL | fn t_f4(...);
| ^^^

error: `...` must be the last argument of a C-variadic function
--> $DIR/variadic-ffi-semantic-restrictions.rs:71:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:84:13
|
LL | fn t_f5(..., x: isize) {}
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:71:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:84:13
|
LL | fn t_f5(..., x: isize) {}
| ^^^

error: `...` must be the last argument of a C-variadic function
--> $DIR/variadic-ffi-semantic-restrictions.rs:74:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:87:13
|
LL | fn t_f6(..., x: isize);
| ^^^

error: only foreign or `unsafe extern "C"` functions may be C-variadic
--> $DIR/variadic-ffi-semantic-restrictions.rs:74:13
--> $DIR/variadic-ffi-semantic-restrictions.rs:87:13
|
LL | fn t_f6(..., x: isize);
| ^^^

error: aborting due to 34 previous errors
error: aborting due to 40 previous errors

0 comments on commit 784f04b

Please sign in to comment.