Skip to content

Commit e784c10

Browse files
Auto merge of #143619 - beetrees:varargs-named, r=<try>
`c_variadic`: Make `fn f(...) {}` error like `fn f(u32) {}` outside of `extern` blocks This PR makes unnamed `...` parameters (such as the one in `unsafe extern "C" fn f(...) {}`) a parse error to be consistent with `unsafe extern "C" fn f(u32) {}`: this is a source of confusion for programmers coming from C, where the `...` parameter is never named and instead calling `va_start` is required; disallowing unnamed `...` parameters also improves the overall consistency of the Rust language by matching the treatment of other unnamed parameters. Unnamed `...` parameters in `extern` blocks (such as `unsafe extern "C" { fn f(...); }`) continue to compile after this PR, as they are already stable and heavily used (and don't cause the mentioned confusion as they are just being used in function declarations). As all the syntax gating for `c_variadic` has been done post-expansion, this is technically a breaking change. In particular, code like this has compiled on stable since Rust 1.35.0: ```rust #[cfg(any())] // Equivalent to the more recent #[cfg(false)] unsafe extern "C" fn bar(_: u32, ...) {} ``` Since this is more or less a stability hole and is unlikely to be used in practice, I think it would be ok to break this, but this will definitely require both a crater check run and a lang FCP. Tracking issue: #44930 cc `@folkertdev` `@workingjubilee` r? `@joshtriplett`
2 parents 040e2f8 + e18dc17 commit e784c10

22 files changed

+348
-166
lines changed

compiler/rustc_parse/src/parser/attr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ impl<'a> Parser<'a> {
200200
AttrWrapper::empty(),
201201
true,
202202
false,
203-
FnParseMode { req_name: |_| true, req_body: true },
203+
FnParseMode { req_name: |_, _| true, req_body: true },
204204
ForceCollect::No,
205205
) {
206206
Ok(Some(item)) => {

compiler/rustc_parse/src/parser/item.rs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl<'a> Parser<'a> {
117117

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

@@ -935,7 +935,7 @@ impl<'a> Parser<'a> {
935935
&mut self,
936936
force_collect: ForceCollect,
937937
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
938-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
938+
let fn_parse_mode = FnParseMode { req_name: |_, _| true, req_body: true };
939939
self.parse_assoc_item(fn_parse_mode, force_collect)
940940
}
941941

@@ -944,7 +944,7 @@ impl<'a> Parser<'a> {
944944
force_collect: ForceCollect,
945945
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
946946
let fn_parse_mode =
947-
FnParseMode { req_name: |edition| edition >= Edition::Edition2018, req_body: false };
947+
FnParseMode { req_name: |edition, _| edition >= Edition::Edition2018, req_body: false };
948948
self.parse_assoc_item(fn_parse_mode, force_collect)
949949
}
950950

@@ -1221,7 +1221,10 @@ impl<'a> Parser<'a> {
12211221
&mut self,
12221222
force_collect: ForceCollect,
12231223
) -> PResult<'a, Option<Option<P<ForeignItem>>>> {
1224-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: false };
1224+
let fn_parse_mode = FnParseMode {
1225+
req_name: |_, is_dot_dot_dot| is_dot_dot_dot == IsDotDotDot::No,
1226+
req_body: false,
1227+
};
12251228
Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
12261229
|Item { attrs, id, span, vis, kind, tokens }| {
12271230
let kind = match ForeignItemKind::try_from(kind) {
@@ -2093,7 +2096,7 @@ impl<'a> Parser<'a> {
20932096
let inherited_vis =
20942097
Visibility { span: DUMMY_SP, kind: VisibilityKind::Inherited, tokens: None };
20952098
// We use `parse_fn` to get a span for the function
2096-
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
2099+
let fn_parse_mode = FnParseMode { req_name: |_, _| true, req_body: true };
20972100
match self.parse_fn(
20982101
&mut AttrVec::new(),
20992102
fn_parse_mode,
@@ -2326,8 +2329,16 @@ impl<'a> Parser<'a> {
23262329
/// The function decides if, per-parameter `p`, `p` must have a pattern or just a type.
23272330
///
23282331
/// This function pointer accepts an edition, because in edition 2015, trait declarations
2329-
/// were allowed to omit parameter names. In 2018, they became required.
2330-
type ReqName = fn(Edition) -> bool;
2332+
/// were allowed to omit parameter names. In 2018, they became required. It also accepts an
2333+
/// `IsDotDotDot` parameter, as `extern` function declarations and function pointer types are
2334+
/// allowed to omit the name of the `...` but regular function items are not.
2335+
type ReqName = fn(Edition, IsDotDotDot) -> bool;
2336+
2337+
#[derive(PartialEq)]
2338+
pub(crate) enum IsDotDotDot {
2339+
Yes,
2340+
No,
2341+
}
23312342

23322343
/// Parsing configuration for functions.
23332344
///
@@ -2360,6 +2371,8 @@ pub(crate) struct FnParseMode {
23602371
/// to true.
23612372
/// * The span is from Edition 2015. In particular, you can get a
23622373
/// 2015 span inside a 2021 crate using macros.
2374+
///
2375+
/// Or if `IsDotDotDot::Yes`, this function will also return `false` with an `extern` block.
23632376
pub(super) req_name: ReqName,
23642377
/// If this flag is set to `true`, then plain, semicolon-terminated function
23652378
/// prototypes are not allowed here.
@@ -2991,10 +3004,15 @@ impl<'a> Parser<'a> {
29913004
return Ok((res?, Trailing::No, UsePreAttrPos::No));
29923005
}
29933006

2994-
let is_name_required = match this.token.kind {
2995-
token::DotDotDot => false,
2996-
_ => req_name(this.token.span.with_neighbor(this.prev_token.span).edition()),
3007+
let is_dot_dot_dot = if this.token.kind == token::DotDotDot {
3008+
IsDotDotDot::Yes
3009+
} else {
3010+
IsDotDotDot::No
29973011
};
3012+
let is_name_required = req_name(
3013+
this.token.span.with_neighbor(this.prev_token.span).edition(),
3014+
is_dot_dot_dot,
3015+
);
29983016
let (pat, ty) = if is_name_required || this.is_named_param() {
29993017
debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required);
30003018
let (pat, colon) = this.parse_fn_param_pat_colon()?;

compiler/rustc_parse/src/parser/path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ impl<'a> Parser<'a> {
400400

401401
let dcx = self.dcx();
402402
let parse_params_result = self.parse_paren_comma_seq(|p| {
403-
let param = p.parse_param_general(|_| false, false, false);
403+
let param = p.parse_param_general(|_, _| false, false, false);
404404
param.map(move |param| {
405405
if !matches!(param.pat.kind, PatKind::Missing) {
406406
dcx.emit_err(FnPathFoundNamedParams {

compiler/rustc_parse/src/parser/stmt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ impl<'a> Parser<'a> {
154154
attrs.clone(), // FIXME: unwanted clone of attrs
155155
false,
156156
true,
157-
FnParseMode { req_name: |_| true, req_body: true },
157+
FnParseMode { req_name: |_, _| true, req_body: true },
158158
force_collect,
159159
)? {
160160
self.mk_stmt(lo.to(item.span), StmtKind::Item(P(item)))

compiler/rustc_parse/src/parser/ty.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ impl<'a> Parser<'a> {
695695
if self.may_recover() && self.token == TokenKind::Lt {
696696
self.recover_fn_ptr_with_generics(lo, &mut params, param_insertion_point)?;
697697
}
698-
let decl = self.parse_fn_decl(|_| false, AllowPlus::No, recover_return_sign)?;
698+
let decl = self.parse_fn_decl(|_, _| false, AllowPlus::No, recover_return_sign)?;
699699

700700
let decl_span = span_start.to(self.prev_token.span);
701701
Ok(TyKind::FnPtr(P(FnPtrTy { ext, safety, generic_params: params, decl, decl_span })))
@@ -1234,7 +1234,7 @@ impl<'a> Parser<'a> {
12341234
self.bump();
12351235
let args_lo = self.token.span;
12361236
let snapshot = self.create_snapshot_for_diagnostic();
1237-
match self.parse_fn_decl(|_| false, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
1237+
match self.parse_fn_decl(|_, _| false, AllowPlus::No, RecoverReturnSign::OnlyFatArrow) {
12381238
Ok(decl) => {
12391239
self.dcx().emit_err(ExpectedFnPathFoundFnKeyword { fn_token_span });
12401240
Some(ast::Path {
@@ -1319,7 +1319,7 @@ impl<'a> Parser<'a> {
13191319
// Parse `(T, U) -> R`.
13201320
let inputs_lo = self.token.span;
13211321
let inputs: ThinVec<_> =
1322-
self.parse_fn_params(|_| false)?.into_iter().map(|input| input.ty).collect();
1322+
self.parse_fn_params(|_, _| false)?.into_iter().map(|input| input.ty).collect();
13231323
let inputs_span = inputs_lo.to(self.prev_token.span);
13241324
let output = self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverReturnSign::No)?;
13251325
let args = ast::ParenthesizedArgs {

tests/crashes/132142.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
//@ known-bug: #132142
22

3-
async extern "cmse-nonsecure-entry" fn fun(...) {}
3+
async extern "cmse-nonsecure-entry" fn fun(_: ...) {}

tests/pretty/hir-fn-variadic.pp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@
1616

1717
fn main() {
1818
fn g1(_: extern "C" fn(_: u8, va: ...)) { }
19-
fn g2(_: extern "C" fn(_: u8, ...)) { }
19+
fn g2(_: extern "C" fn(_: u8, _: ...)) { }
2020
fn g3(_: extern "C" fn(u8, va: ...)) { }
21-
fn g4(_: extern "C" fn(u8, ...)) { }
21+
fn g4(_: extern "C" fn(u8, _: ...)) { }
2222

2323
fn g5(_: extern "C" fn(va: ...)) { }
24-
fn g6(_: extern "C" fn(...)) { }
24+
fn g6(_: extern "C" fn(_: ...)) { }
2525

2626
{
2727
let _ =

tests/pretty/hir-fn-variadic.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@ pub unsafe extern "C" fn bar(_: i32, mut va2: ...) -> usize {
1414

1515
fn main() {
1616
fn g1(_: extern "C" fn(_: u8, va: ...)) {}
17-
fn g2(_: extern "C" fn(_: u8, ...)) {}
17+
fn g2(_: extern "C" fn(_: u8, _: ...)) {}
1818
fn g3(_: extern "C" fn(u8, va: ...)) {}
19-
fn g4(_: extern "C" fn(u8, ...)) {}
19+
fn g4(_: extern "C" fn(u8, _: ...)) {}
2020

2121
fn g5(_: extern "C" fn(va: ...)) {}
22-
fn g6(_: extern "C" fn(...)) {}
22+
fn g6(_: extern "C" fn(_: ...)) {}
2323

2424
_ = { unsafe extern "C" fn f1(_: u8, va: ...) {} };
25-
_ = { unsafe extern "C" fn f2(_: u8, ...) {} };
25+
_ = { unsafe extern "C" fn f2(_: u8, _: ...) {} };
2626

2727
_ = { unsafe extern "C" fn f5(va: ...) {} };
28-
_ = { unsafe extern "C" fn f6(...) {} };
28+
_ = { unsafe extern "C" fn f6(_: ...) {} };
2929
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ fn ordering4 < 'a , 'b > ( a : , self , self , self ,
88
//~| ERROR unexpected `self` parameter in function
99
//~| ERROR unexpected `self` parameter in function
1010
//~| ERROR unexpected `self` parameter in function
11-
self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
11+
self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
1212
//~^ ERROR unexpected `self` parameter in function
1313
//~| ERROR unexpected `self` parameter in function
1414
//~| ERROR unexpected `self` parameter in function

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,46 +25,46 @@ LL | fn ordering4 < 'a , 'b > ( a : , self , self , self ,
2525
error: unexpected `self` parameter in function
2626
--> $DIR/issue-86053-1.rs:11:5
2727
|
28-
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
28+
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
2929
| ^^^^ must be the first parameter of an associated function
3030

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

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

4343
error: `...` must be the last argument of a C-variadic function
4444
--> $DIR/issue-86053-1.rs:11:12
4545
|
46-
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
47-
| ^^^
46+
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
47+
| ^^^^^^
4848

4949
error: only foreign, `unsafe extern "C"`, or `unsafe extern "C-unwind"` functions may have a C-variadic arg
5050
--> $DIR/issue-86053-1.rs:11:12
5151
|
52-
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
53-
| ^^^ ^^^
52+
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
53+
| ^^^^^^ ^^^^^^
5454

5555
error[E0412]: cannot find type `F` in this scope
56-
--> $DIR/issue-86053-1.rs:11:48
56+
--> $DIR/issue-86053-1.rs:11:54
5757
|
58-
LL | self , ... , self , self , ... ) where F : FnOnce ( & 'a & 'b usize ) {
59-
| ^
58+
LL | self , _: ... , self , self , _: ... ) where F : FnOnce ( & 'a & 'b usize ) {
59+
| ^
6060
--> $SRC_DIR/core/src/ops/function.rs:LL:COL
6161
|
6262
= note: similarly named trait `Fn` defined here
6363
|
6464
help: a trait with a similar name exists
6565
|
66-
LL | self , ... , self , self , ... ) where Fn : FnOnce ( & 'a & 'b usize ) {
67-
| +
66+
LL | self , _: ... , self , self , _: ... ) where Fn : FnOnce ( & 'a & 'b usize ) {
67+
| +
6868
help: you might be missing a type parameter
6969
|
7070
LL | fn ordering4 < 'a , 'b, F > ( a : , self , self , self ,

0 commit comments

Comments
 (0)