Skip to content

c_variadic: Add future-incompatibility warning for ... arguments without a pattern outside of extern blocks #143619

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 1 commit 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
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -1051,5 +1051,8 @@ lint_useless_ptr_null_checks_ref = references are not nullable, so checking them

lint_uses_power_alignment = repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type

lint_varargs_without_pattern = missing pattern for `...` argument
.suggestion = name the argument, or use `_` to continue ignoring it

lint_variant_size_differences =
enum variant is more than three times larger ({$largest} bytes) than the next largest
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/early/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,5 +468,8 @@ pub fn decorate_builtin_lint(
BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => {
lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag)
}
BuiltinLintDiag::VarargsWithoutPattern { span } => {
lints::VarargsWithoutPattern { span }.decorate_lint(diag)
}
}
}
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3215,6 +3215,13 @@ pub(crate) struct ReservedMultihash {
pub suggestion: Span,
}

#[derive(LintDiagnostic)]
#[diag(lint_varargs_without_pattern)]
pub(crate) struct VarargsWithoutPattern {
#[suggestion(code = "_: ...", applicability = "machine-applicable")]
pub span: Span,
}

#[derive(Debug)]
pub(crate) struct MismatchedLifetimeSyntaxes {
pub inputs: LifetimeSyntaxCategories<Vec<Span>>,
Expand Down
48 changes: 48 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ declare_lint_pass! {
UNUSED_UNSAFE,
UNUSED_VARIABLES,
USELESS_DEPRECATED,
VARARGS_WITHOUT_PATTERN,
WARNINGS,
// tidy-alphabetical-end
]
Expand Down Expand Up @@ -5099,3 +5100,50 @@ declare_lint! {
report_in_deps: true,
};
}

declare_lint! {
/// The `varargs_without_pattern` lint detects when `...` is used as an argument to a
/// non-foreign function without any pattern being specified.
///
/// ### Example
///
/// ```rust
/// // Using `...` in non-foreign function definitions is unstable, however stability is
/// // currently only checked after attributes are expanded, so using `#[cfg(false)]` here will
/// // allow this to compile on stable Rust.
/// #[cfg(false)]
/// fn foo(...) {
///
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Patterns are currently required for all non-`...` arguments in function definitions (with
/// some exceptions in the 2015 edition). Requiring `...` arguments to have patterns in
/// non-foreign function defitions makes the language more consistent, and removes a source of
/// confusion for the unstable C variadic feature. `...` arguments without a pattern are already
/// stable and widely used in foreign function definitions; this lint only affects non-foreign
/// function defitions.
///
/// Using `...` (C varargs) in a non-foreign function definition is currently unstable. However,
/// stability checking for the `...` syntax in non-foreign function definitions is currently
/// implemented after attributes have been expanded, meaning that if the attribute removes the
/// use of the unstable syntax (e.g. `#[cfg(false)]`, or a procedural macro), the code will
/// compile on stable Rust; this is the only situtation where this lint affects code that
/// compiles on stable Rust.
///
/// This is a [future-incompatible] lint to transition this to a hard error in the future.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub VARARGS_WITHOUT_PATTERN,
Warn,
"detects usage of `...` arguments without a pattern in non-foreign items",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseError,
reference: "issue #FIXME <https://github.com/rust-lang/rust/issues/FIXME>",
report_in_deps: false,
};
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,9 @@ pub enum BuiltinLintDiag {
cfg_name: Symbol,
controlled_by: &'static str,
},
VarargsWithoutPattern {
span: Span,
},
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl<'a> Parser<'a> {
AttrWrapper::empty(),
true,
false,
FnParseMode { req_name: |_| true, req_body: true },
FnParseMode { req_name: |_, _| true, req_body: true },
ForceCollect::No,
) {
Ok(Some(item)) => {
Expand Down
51 changes: 41 additions & 10 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use rustc_ast::{self as ast};
use rustc_ast_pretty::pprust;
use rustc_errors::codes::*;
use rustc_errors::{Applicability, PResult, StashKey, struct_span_code_err};
use rustc_session::lint::BuiltinLintDiag;
use rustc_session::lint::builtin::VARARGS_WITHOUT_PATTERN;
use rustc_span::edit_distance::edit_distance;
use rustc_span::edition::Edition;
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Ident, Span, Symbol, kw, source_map, sym};
Expand Down Expand Up @@ -117,7 +119,7 @@ impl<'a> Parser<'a> {

impl<'a> Parser<'a> {
pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<P<Item>>> {
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
let fn_parse_mode = FnParseMode { req_name: |_, _| true, req_body: true };
self.parse_item_(fn_parse_mode, force_collect).map(|i| i.map(P))
}

Expand Down Expand Up @@ -949,7 +951,7 @@ impl<'a> Parser<'a> {
&mut self,
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
let fn_parse_mode = FnParseMode { req_name: |_, _| true, req_body: true };
self.parse_assoc_item(fn_parse_mode, force_collect)
}

Expand All @@ -958,7 +960,7 @@ impl<'a> Parser<'a> {
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
let fn_parse_mode =
FnParseMode { req_name: |edition| edition >= Edition::Edition2018, req_body: false };
FnParseMode { req_name: |edition, _| edition >= Edition::Edition2018, req_body: false };
self.parse_assoc_item(fn_parse_mode, force_collect)
}

Expand Down Expand Up @@ -1235,7 +1237,10 @@ impl<'a> Parser<'a> {
&mut self,
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<ForeignItem>>>> {
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: false };
let fn_parse_mode = FnParseMode {
req_name: |_, is_dot_dot_dot| is_dot_dot_dot == IsDotDotDot::No,
req_body: false,
};
Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
|Item { attrs, id, span, vis, kind, tokens }| {
let kind = match ForeignItemKind::try_from(kind) {
Expand Down Expand Up @@ -2107,7 +2112,7 @@ impl<'a> Parser<'a> {
let inherited_vis =
Visibility { span: DUMMY_SP, kind: VisibilityKind::Inherited, tokens: None };
// We use `parse_fn` to get a span for the function
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
let fn_parse_mode = FnParseMode { req_name: |_, _| true, req_body: true };
match self.parse_fn(
&mut AttrVec::new(),
fn_parse_mode,
Expand Down Expand Up @@ -2340,8 +2345,16 @@ impl<'a> Parser<'a> {
/// The function decides if, per-parameter `p`, `p` must have a pattern or just a type.
///
/// This function pointer accepts an edition, because in edition 2015, trait declarations
/// were allowed to omit parameter names. In 2018, they became required.
type ReqName = fn(Edition) -> bool;
/// were allowed to omit parameter names. In 2018, they became required. It also accepts an
/// `IsDotDotDot` parameter, as `extern` function declarations and function pointer types are
/// allowed to omit the name of the `...` but regular function items are not.
type ReqName = fn(Edition, IsDotDotDot) -> bool;

#[derive(Copy, Clone, PartialEq)]
pub(crate) enum IsDotDotDot {
Yes,
No,
}

/// Parsing configuration for functions.
///
Expand Down Expand Up @@ -2374,6 +2387,8 @@ pub(crate) struct FnParseMode {
/// to true.
/// * The span is from Edition 2015. In particular, you can get a
/// 2015 span inside a 2021 crate using macros.
///
/// Or if `IsDotDotDot::Yes`, this function will also return `false` with an `extern` block.
pub(super) req_name: ReqName,
/// If this flag is set to `true`, then plain, semicolon-terminated function
/// prototypes are not allowed here.
Expand Down Expand Up @@ -3005,9 +3020,25 @@ impl<'a> Parser<'a> {
return Ok((res?, Trailing::No, UsePreAttrPos::No));
}

let is_name_required = match this.token.kind {
token::DotDotDot => false,
_ => req_name(this.token.span.with_neighbor(this.prev_token.span).edition()),
let is_dot_dot_dot = if this.token.kind == token::DotDotDot {
IsDotDotDot::Yes
} else {
IsDotDotDot::No
};
let is_name_required = req_name(
this.token.span.with_neighbor(this.prev_token.span).edition(),
is_dot_dot_dot,
);
let is_name_required = if is_name_required && is_dot_dot_dot == IsDotDotDot::Yes {
this.psess.buffer_lint(
VARARGS_WITHOUT_PATTERN,
this.token.span,
ast::CRATE_NODE_ID,
BuiltinLintDiag::VarargsWithoutPattern { span: this.token.span },
);
false
} else {
is_name_required
};
let (pat, ty) = if is_name_required || this.is_named_param() {
debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ impl<'a> Parser<'a> {

let dcx = self.dcx();
let parse_params_result = self.parse_paren_comma_seq(|p| {
let param = p.parse_param_general(|_| false, false, false);
let param = p.parse_param_general(|_, _| false, false, false);
param.map(move |param| {
if !matches!(param.pat.kind, PatKind::Missing) {
dcx.emit_err(FnPathFoundNamedParams {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl<'a> Parser<'a> {
attrs.clone(), // FIXME: unwanted clone of attrs
false,
true,
FnParseMode { req_name: |_| true, req_body: true },
FnParseMode { req_name: |_, _| true, req_body: true },
force_collect,
)? {
self.mk_stmt(lo.to(item.span), StmtKind::Item(P(item)))
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ impl<'a> Parser<'a> {
if self.may_recover() && self.token == TokenKind::Lt {
self.recover_fn_ptr_with_generics(lo, &mut params, param_insertion_point)?;
}
let decl = self.parse_fn_decl(|_| false, AllowPlus::No, recover_return_sign)?;
let decl = self.parse_fn_decl(|_, _| false, AllowPlus::No, recover_return_sign)?;

let decl_span = span_start.to(self.prev_token.span);
Ok(TyKind::FnPtr(P(FnPtrTy { ext, safety, generic_params: params, decl, decl_span })))
Expand Down Expand Up @@ -1289,7 +1289,7 @@ impl<'a> Parser<'a> {
self.bump();
let args_lo = self.token.span;
let snapshot = self.create_snapshot_for_diagnostic();
match self.parse_fn_decl(|_| false, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
match self.parse_fn_decl(|_, _| false, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
Ok(decl) => {
self.dcx().emit_err(ExpectedFnPathFoundFnKeyword { fn_token_span });
Some(ast::Path {
Expand Down Expand Up @@ -1374,7 +1374,7 @@ impl<'a> Parser<'a> {
// Parse `(T, U) -> R`.
let inputs_lo = self.token.span;
let inputs: ThinVec<_> =
self.parse_fn_params(|_| false)?.into_iter().map(|input| input.ty).collect();
self.parse_fn_params(|_, _| false)?.into_iter().map(|input| input.ty).collect();
let inputs_span = inputs_lo.to(self.prev_token.span);
let output = self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverReturnSign::No)?;
let args = ast::ParenthesizedArgs {
Expand Down
2 changes: 1 addition & 1 deletion tests/crashes/132142.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
//@ known-bug: #132142

async extern "cmse-nonsecure-entry" fn fun(...) {}
async extern "cmse-nonsecure-entry" fn fun(_: ...) {}
6 changes: 3 additions & 3 deletions tests/pretty/hir-fn-variadic.pp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@

fn main() {
fn g1(_: extern "C" fn(_: u8, va: ...)) { }
fn g2(_: extern "C" fn(_: u8, ...)) { }
fn g2(_: extern "C" fn(_: u8, _: ...)) { }
fn g3(_: extern "C" fn(u8, va: ...)) { }
fn g4(_: extern "C" fn(u8, ...)) { }
fn g4(_: extern "C" fn(u8, _: ...)) { }

fn g5(_: extern "C" fn(va: ...)) { }
fn g6(_: extern "C" fn(...)) { }
fn g6(_: extern "C" fn(_: ...)) { }

{
let _ =
Expand Down
10 changes: 5 additions & 5 deletions tests/pretty/hir-fn-variadic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ pub unsafe extern "C" fn bar(_: i32, mut va2: ...) -> usize {

fn main() {
fn g1(_: extern "C" fn(_: u8, va: ...)) {}
fn g2(_: extern "C" fn(_: u8, ...)) {}
fn g2(_: extern "C" fn(_: u8, _: ...)) {}
fn g3(_: extern "C" fn(u8, va: ...)) {}
fn g4(_: extern "C" fn(u8, ...)) {}
fn g4(_: extern "C" fn(u8, _: ...)) {}

fn g5(_: extern "C" fn(va: ...)) {}
fn g6(_: extern "C" fn(...)) {}
fn g6(_: extern "C" fn(_: ...)) {}

_ = { unsafe extern "C" fn f1(_: u8, va: ...) {} };
_ = { unsafe extern "C" fn f2(_: u8, ...) {} };
_ = { unsafe extern "C" fn f2(_: u8, _: ...) {} };

_ = { unsafe extern "C" fn f5(va: ...) {} };
_ = { unsafe extern "C" fn f6(...) {} };
_ = { unsafe extern "C" fn f6(_: ...) {} };
}
2 changes: 1 addition & 1 deletion tests/ui/c-variadic/issue-86053-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn ordering4 < 'a , 'b > ( a : , self , self , self ,
//~| ERROR unexpected `self` parameter in function
//~| ERROR unexpected `self` parameter in function
//~| ERROR unexpected `self` parameter in function
self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
//~^ ERROR unexpected `self` parameter in function
//~| ERROR unexpected `self` parameter in function
//~| ERROR unexpected `self` parameter in function
Expand Down
32 changes: 16 additions & 16 deletions tests/ui/c-variadic/issue-86053-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,46 +25,46 @@ LL | fn ordering4 < 'a , 'b > ( a : , self , self , self ,
error: unexpected `self` parameter in function
--> $DIR/issue-86053-1.rs:11:5
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^^ must be the first parameter of an associated function

error: unexpected `self` parameter in function
--> $DIR/issue-86053-1.rs:11:20
--> $DIR/issue-86053-1.rs:11:23
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^^ must be the first parameter of an associated function
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^^ must be the first parameter of an associated function

error: unexpected `self` parameter in function
--> $DIR/issue-86053-1.rs:11:29
--> $DIR/issue-86053-1.rs:11:32
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^^ must be the first parameter of an associated function
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^^ must be the first parameter of an associated function

error: `...` must be the last argument of a C-variadic function
--> $DIR/issue-86053-1.rs:11:12
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^
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
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^^^ ^^^
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
--> $DIR/issue-86053-1.rs:11:54
|
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
| ^
--> $SRC_DIR/core/src/ops/function.rs:LL:COL
|
= note: similarly named trait `Fn` defined here
|
help: a trait with a similar name exists
|
LL | self , ... , self , self , ... ) where Fn : FnOnce ( & 'a & 'b usize ) {
| +
LL | self , _: ... , self , self , _: ... ) where Fn : FnOnce ( & 'a & 'b usize ) {
| +
help: you might be missing a type parameter
|
LL | fn ordering4 < 'a , 'b, F > ( a : , self , self , self ,
Expand Down
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