From 8508e658958d02bc0c4bb62646a9cf9098ee53a8 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sun, 29 Oct 2023 17:04:52 -0400 Subject: [PATCH 1/2] Fix 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. --- .../rustc_ast_passes/src/ast_validation.rs | 18 +++++--- compiler/rustc_ast_passes/src/errors.rs | 2 +- tests/ui/c-variadic/issue-86053-1.stderr | 10 +---- .../variadic-ffi-semantic-restrictions.rs | 2 - .../variadic-ffi-semantic-restrictions.stderr | 42 +++++++------------ 5 files changed, 31 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 0477c7b2ba99c..f59f7d389cdab 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -485,6 +485,18 @@ impl<'a> AstValidator<'a> { /// Reject C-variadic type unless the function is 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; + } + match (fk.ctxt(), fk.header()) { (Some(FnCtxt::Foreign), _) => return, (Some(FnCtxt::Free), Some(header)) => match header.ext { @@ -499,11 +511,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) { diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index e74d94e43474d..579f528e20f94 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -271,7 +271,7 @@ pub struct ExternItemAscii { #[diag(ast_passes_bad_c_variadic)] pub struct BadCVariadic { #[primary_span] - pub span: Span, + pub span: Vec, } #[derive(Diagnostic)] diff --git a/tests/ui/c-variadic/issue-86053-1.stderr b/tests/ui/c-variadic/issue-86053-1.stderr index 5a02f4aa93a95..69e19e1d4d26b 100644 --- a/tests/ui/c-variadic/issue-86053-1.stderr +++ b/tests/ui/c-variadic/issue-86053-1.stderr @@ -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 @@ -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`. diff --git a/tests/ui/parser/variadic-ffi-semantic-restrictions.rs b/tests/ui/parser/variadic-ffi-semantic-restrictions.rs index 0b61e267da80b..8e382314ca27e 100644 --- a/tests/ui/parser/variadic-ffi-semantic-restrictions.rs +++ b/tests/ui/parser/variadic-ffi-semantic-restrictions.rs @@ -49,11 +49,9 @@ 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 } diff --git a/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr b/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr index f1cbbb279c849..95e381f06fc46 100644 --- a/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr +++ b/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr @@ -116,91 +116,79 @@ error: only foreign or `unsafe extern "C"` functions may be C-variadic --> $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:28 - | -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 - | -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:53: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:28 + --> $DIR/variadic-ffi-semantic-restrictions.rs:53: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:61:23 + --> $DIR/variadic-ffi-semantic-restrictions.rs:59: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:61: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:63: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:63: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:66: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:66: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:69: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:69: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:72: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:72:13 | LL | fn t_f6(..., x: isize); | ^^^ -error: aborting due to 34 previous errors +error: aborting due to 32 previous errors From f91b5ceaf2568c38afc5a4b7c82459ca3d857998 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sun, 29 Oct 2023 17:28:56 -0400 Subject: [PATCH 2/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". --- compiler/rustc_ast_passes/messages.ftl | 4 + .../rustc_ast_passes/src/ast_validation.rs | 19 +++- compiler/rustc_ast_passes/src/errors.rs | 11 +++ .../variadic-ffi-semantic-restrictions.rs | 15 +++ .../variadic-ffi-semantic-restrictions.stderr | 93 +++++++++++++++---- 5 files changed, 120 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl index 43020a930788f..d22bae816ef41 100644 --- a/compiler/rustc_ast_passes/messages.ftl +++ b/compiler/rustc_ast_passes/messages.ftl @@ -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 diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index f59f7d389cdab..477d3732d464f 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -482,8 +482,11 @@ 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() @@ -497,6 +500,18 @@ impl<'a> AstValidator<'a> { 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 { diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 579f528e20f94..d14b62d6bdc66 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -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, + #[label(ast_passes_const)] + pub const_span: Span, + #[label(ast_passes_variadic)] + pub variadic_spans: Vec, +} + #[derive(Diagnostic)] #[diag(ast_passes_pattern_in_foreign, code = "E0130")] pub struct PatternInForeign { diff --git a/tests/ui/parser/variadic-ffi-semantic-restrictions.rs b/tests/ui/parser/variadic-ffi-semantic-restrictions.rs index 8e382314ca27e..b173e23e7a183 100644 --- a/tests/ui/parser/variadic-ffi-semantic-restrictions.rs +++ b/tests/ui/parser/variadic-ffi-semantic-restrictions.rs @@ -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 @@ -53,6 +65,9 @@ impl X { fn i_f4(..., 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 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 { diff --git a/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr b/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr index 95e381f06fc46..18526080e4cb7 100644 --- a/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr +++ b/tests/ui/parser/variadic-ffi-semantic-restrictions.stderr @@ -76,119 +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 + --> $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:13 + --> $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:53: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:53: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:68:29 + | +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:59: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:61: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:63: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:63: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:66: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:66: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:69: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:69: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:72: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:72:13 + --> $DIR/variadic-ffi-semantic-restrictions.rs:87:13 | LL | fn t_f6(..., x: isize); | ^^^ -error: aborting due to 32 previous errors +error: aborting due to 40 previous errors