Skip to content

Commit 3548be9

Browse files
committed
Gracefully handle confusing -> with : in function return type
1 parent 5404dee commit 3548be9

18 files changed

+205
-79
lines changed

compiler/rustc_parse/src/parser/expr.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::pat::{GateOr, PARAM_EXPECTED};
2-
use super::ty::{AllowPlus, RecoverFatArrow, RecoverQPath};
2+
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
33
use super::{BlockMode, Parser, PathStyle, Restrictions, TokenType};
44
use super::{SemiColonMode, SeqSep, TokenExpectType};
55
use crate::maybe_recover_from_interpolated_ty_qpath;
@@ -1647,7 +1647,8 @@ impl<'a> Parser<'a> {
16471647
self.expect_or()?;
16481648
args
16491649
};
1650-
let output = self.parse_ret_ty(AllowPlus::Yes, RecoverQPath::Yes, RecoverFatArrow::Yes)?;
1650+
let output =
1651+
self.parse_ret_ty(AllowPlus::Yes, RecoverQPath::Yes, RecoverReturnSign::Yes)?;
16511652

16521653
Ok(P(FnDecl { inputs, output }))
16531654
}

compiler/rustc_parse/src/parser/generics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ impl<'a> Parser<'a> {
240240

241241
// Parse type with mandatory colon and (possibly empty) bounds,
242242
// or with mandatory equality sign and the second type.
243-
let ty = self.parse_ty()?;
243+
let ty = self.parse_ty_for_where_clause()?;
244244
if self.eat(&token::Colon) {
245245
let bounds = self.parse_generic_bounds(Some(self.prev_token.span))?;
246246
Ok(ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate {

compiler/rustc_parse/src/parser/item.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::diagnostics::{dummy_arg, ConsumeClosingDelim, Error};
2-
use super::ty::{AllowPlus, RecoverFatArrow, RecoverQPath};
2+
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
33
use super::{FollowedByType, Parser, PathStyle};
44

55
use crate::maybe_whole;
@@ -1514,7 +1514,7 @@ impl<'a> Parser<'a> {
15141514
let header = self.parse_fn_front_matter()?; // `const ... fn`
15151515
let ident = self.parse_ident()?; // `foo`
15161516
let mut generics = self.parse_generics()?; // `<'a, T, ...>`
1517-
let decl = self.parse_fn_decl(req_name, AllowPlus::Yes)?; // `(p: u8, ...)`
1517+
let decl = self.parse_fn_decl(req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
15181518
generics.where_clause = self.parse_where_clause()?; // `where T: Ord`
15191519

15201520
let mut sig_hi = self.prev_token.span;
@@ -1645,10 +1645,11 @@ impl<'a> Parser<'a> {
16451645
&mut self,
16461646
req_name: ReqName,
16471647
ret_allow_plus: AllowPlus,
1648+
recover_return_sign: RecoverReturnSign,
16481649
) -> PResult<'a, P<FnDecl>> {
16491650
Ok(P(FnDecl {
16501651
inputs: self.parse_fn_params(req_name)?,
1651-
output: self.parse_ret_ty(ret_allow_plus, RecoverQPath::Yes, RecoverFatArrow::Yes)?,
1652+
output: self.parse_ret_ty(ret_allow_plus, RecoverQPath::Yes, recover_return_sign)?,
16521653
}))
16531654
}
16541655

compiler/rustc_parse/src/parser/path.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::ty::{AllowPlus, RecoverFatArrow, RecoverQPath};
1+
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
22
use super::{Parser, TokenType};
33
use crate::maybe_whole;
44
use rustc_ast::ptr::P;
@@ -232,7 +232,7 @@ impl<'a> Parser<'a> {
232232
let (inputs, _) = self.parse_paren_comma_seq(|p| p.parse_ty())?;
233233
let span = ident.span.to(self.prev_token.span);
234234
let output =
235-
self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverFatArrow::No)?;
235+
self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverReturnSign::No)?;
236236
ParenthesizedArgs { inputs, output, span }.into()
237237
};
238238

compiler/rustc_parse/src/parser/ty.rs

+65-13
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,23 @@ pub(super) enum RecoverQPath {
4343
No,
4444
}
4545

46-
#[derive(PartialEq)]
47-
pub(super) enum RecoverFatArrow {
46+
#[derive(Copy, Clone, PartialEq)]
47+
pub(super) enum RecoverReturnSign {
4848
Yes,
49+
OnlyFatArrow,
4950
No,
5051
}
5152

53+
impl RecoverReturnSign {
54+
fn can_recover(self, token: &TokenKind) -> bool {
55+
match self {
56+
Self::Yes => matches!(token, token::FatArrow | token::Colon),
57+
Self::OnlyFatArrow => matches!(token, token::FatArrow),
58+
Self::No => false,
59+
}
60+
}
61+
}
62+
5263
// Is `...` (`CVarArgs`) legal at this level of type parsing?
5364
#[derive(PartialEq)]
5465
enum AllowCVariadic {
@@ -68,14 +79,24 @@ fn can_continue_type_after_non_fn_ident(t: &Token) -> bool {
6879
impl<'a> Parser<'a> {
6980
/// Parses a type.
7081
pub fn parse_ty(&mut self) -> PResult<'a, P<Ty>> {
71-
self.parse_ty_common(AllowPlus::Yes, RecoverQPath::Yes, AllowCVariadic::No)
82+
self.parse_ty_common(
83+
AllowPlus::Yes,
84+
RecoverQPath::Yes,
85+
AllowCVariadic::No,
86+
RecoverReturnSign::Yes,
87+
)
7288
}
7389

7490
/// Parse a type suitable for a function or function pointer parameter.
7591
/// The difference from `parse_ty` is that this version allows `...`
7692
/// (`CVarArgs`) at the top level of the type.
7793
pub(super) fn parse_ty_for_param(&mut self) -> PResult<'a, P<Ty>> {
78-
self.parse_ty_common(AllowPlus::Yes, RecoverQPath::Yes, AllowCVariadic::Yes)
94+
self.parse_ty_common(
95+
AllowPlus::Yes,
96+
RecoverQPath::Yes,
97+
AllowCVariadic::Yes,
98+
RecoverReturnSign::Yes,
99+
)
79100
}
80101

81102
/// Parses a type in restricted contexts where `+` is not permitted.
@@ -85,21 +106,41 @@ impl<'a> Parser<'a> {
85106
/// Example 2: `value1 as TYPE + value2`
86107
/// `+` is prohibited to avoid interactions with expression grammar.
87108
pub(super) fn parse_ty_no_plus(&mut self) -> PResult<'a, P<Ty>> {
88-
self.parse_ty_common(AllowPlus::No, RecoverQPath::Yes, AllowCVariadic::No)
109+
self.parse_ty_common(
110+
AllowPlus::No,
111+
RecoverQPath::Yes,
112+
AllowCVariadic::No,
113+
RecoverReturnSign::Yes,
114+
)
115+
}
116+
117+
/// Parse a type without recovering `:` as `->` to avoid breaking code such as `where fn() : for<'a>`
118+
pub(super) fn parse_ty_for_where_clause(&mut self) -> PResult<'a, P<Ty>> {
119+
self.parse_ty_common(
120+
AllowPlus::Yes,
121+
RecoverQPath::Yes,
122+
AllowCVariadic::Yes,
123+
RecoverReturnSign::OnlyFatArrow,
124+
)
89125
}
90126

91127
/// Parses an optional return type `[ -> TY ]` in a function declaration.
92128
pub(super) fn parse_ret_ty(
93129
&mut self,
94130
allow_plus: AllowPlus,
95131
recover_qpath: RecoverQPath,
96-
recover_fat_arrow: RecoverFatArrow,
132+
recover_return_sign: RecoverReturnSign,
97133
) -> PResult<'a, FnRetTy> {
98134
Ok(if self.eat(&token::RArrow) {
99135
// FIXME(Centril): Can we unconditionally `allow_plus`?
100-
let ty = self.parse_ty_common(allow_plus, recover_qpath, AllowCVariadic::No)?;
136+
let ty = self.parse_ty_common(
137+
allow_plus,
138+
recover_qpath,
139+
AllowCVariadic::No,
140+
recover_return_sign,
141+
)?;
101142
FnRetTy::Ty(ty)
102-
} else if recover_fat_arrow == RecoverFatArrow::Yes && self.token == token::FatArrow {
143+
} else if recover_return_sign.can_recover(&self.token.kind) {
103144
// Don't `eat` to prevent `=>` from being added as an expected token which isn't
104145
// actually expected and could only confuse users
105146
self.bump();
@@ -111,7 +152,12 @@ impl<'a> Parser<'a> {
111152
Applicability::MachineApplicable,
112153
)
113154
.emit();
114-
let ty = self.parse_ty_common(allow_plus, recover_qpath, AllowCVariadic::No)?;
155+
let ty = self.parse_ty_common(
156+
allow_plus,
157+
recover_qpath,
158+
AllowCVariadic::No,
159+
recover_return_sign,
160+
)?;
115161
FnRetTy::Ty(ty)
116162
} else {
117163
FnRetTy::Default(self.token.span.shrink_to_lo())
@@ -123,6 +169,7 @@ impl<'a> Parser<'a> {
123169
allow_plus: AllowPlus,
124170
recover_qpath: RecoverQPath,
125171
allow_c_variadic: AllowCVariadic,
172+
recover_return_sign: RecoverReturnSign,
126173
) -> PResult<'a, P<Ty>> {
127174
let allow_qpath_recovery = recover_qpath == RecoverQPath::Yes;
128175
maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery);
@@ -150,14 +197,14 @@ impl<'a> Parser<'a> {
150197
TyKind::Infer
151198
} else if self.check_fn_front_matter() {
152199
// Function pointer type
153-
self.parse_ty_bare_fn(lo, Vec::new())?
200+
self.parse_ty_bare_fn(lo, Vec::new(), recover_return_sign)?
154201
} else if self.check_keyword(kw::For) {
155202
// Function pointer type or bound list (trait object type) starting with a poly-trait.
156203
// `for<'lt> [unsafe] [extern "ABI"] fn (&'lt S) -> T`
157204
// `for<'lt> Trait1<'lt> + Trait2 + 'a`
158205
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
159206
if self.check_fn_front_matter() {
160-
self.parse_ty_bare_fn(lo, lifetime_defs)?
207+
self.parse_ty_bare_fn(lo, lifetime_defs, recover_return_sign)?
161208
} else {
162209
let path = self.parse_path(PathStyle::Type)?;
163210
let parse_plus = allow_plus == AllowPlus::Yes && self.check_plus();
@@ -359,9 +406,14 @@ impl<'a> Parser<'a> {
359406
/// Function Style ABI Parameter types
360407
/// ```
361408
/// We actually parse `FnHeader FnDecl`, but we error on `const` and `async` qualifiers.
362-
fn parse_ty_bare_fn(&mut self, lo: Span, params: Vec<GenericParam>) -> PResult<'a, TyKind> {
409+
fn parse_ty_bare_fn(
410+
&mut self,
411+
lo: Span,
412+
params: Vec<GenericParam>,
413+
recover_return_sign: RecoverReturnSign,
414+
) -> PResult<'a, TyKind> {
363415
let ast::FnHeader { ext, unsafety, constness, asyncness } = self.parse_fn_front_matter()?;
364-
let decl = self.parse_fn_decl(|_| false, AllowPlus::No)?;
416+
let decl = self.parse_fn_decl(|_| false, AllowPlus::No, recover_return_sign)?;
365417
let whole_span = lo.to(self.prev_token.span);
366418
if let ast::Const::Yes(span) = constness {
367419
self.error_fn_ptr_bad_qualifier(whole_span, span, "const");

src/test/ui/fn/fn-fat-arrow-return.fixed

-18
This file was deleted.

src/test/ui/fn/fn-fat-arrow-return.stderr

-8
This file was deleted.

src/test/ui/fn/fn-fat-arrow-return2.rs

-10
This file was deleted.

src/test/ui/fn/fn-fat-arrow-return2.stderr

-14
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// run-rustfix
2+
#![allow(unused)]
3+
fn a() -> usize { 0 }
4+
//~^ ERROR return types are denoted using `->`
5+
6+
fn b()-> usize { 0 }
7+
//~^ ERROR return types are denoted using `->`
8+
9+
fn bar(_: u32) {}
10+
11+
fn baz() -> *const dyn Fn(u32) { unimplemented!() }
12+
13+
fn foo() {
14+
match () {
15+
_ if baz() == &bar as &dyn Fn(u32) => (),
16+
() => (),
17+
}
18+
}
19+
20+
fn main() {
21+
let foo = |a: bool| -> bool { a };
22+
//~^ ERROR return types are denoted using `->`
23+
dbg!(foo(false));
24+
25+
let bar = |a: bool|-> bool { a };
26+
//~^ ERROR return types are denoted using `->`
27+
dbg!(bar(false));
28+
}

src/test/ui/fn/fn-fat-arrow-return.rs src/test/ui/fn/fn-recover-return-sign.rs

+10
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
fn a() => usize { 0 }
44
//~^ ERROR return types are denoted using `->`
55

6+
fn b(): usize { 0 }
7+
//~^ ERROR return types are denoted using `->`
8+
69
fn bar(_: u32) {}
710

811
fn baz() -> *const dyn Fn(u32) { unimplemented!() }
@@ -15,4 +18,11 @@ fn foo() {
1518
}
1619

1720
fn main() {
21+
let foo = |a: bool| => bool { a };
22+
//~^ ERROR return types are denoted using `->`
23+
dbg!(foo(false));
24+
25+
let bar = |a: bool|: bool { a };
26+
//~^ ERROR return types are denoted using `->`
27+
dbg!(bar(false));
1828
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: return types are denoted using `->`
2+
--> $DIR/fn-recover-return-sign.rs:3:8
3+
|
4+
LL | fn a() => usize { 0 }
5+
| ^^ help: use `->` instead
6+
7+
error: return types are denoted using `->`
8+
--> $DIR/fn-recover-return-sign.rs:6:7
9+
|
10+
LL | fn b(): usize { 0 }
11+
| ^ help: use `->` instead
12+
13+
error: return types are denoted using `->`
14+
--> $DIR/fn-recover-return-sign.rs:21:25
15+
|
16+
LL | let foo = |a: bool| => bool { a };
17+
| ^^ help: use `->` instead
18+
19+
error: return types are denoted using `->`
20+
--> $DIR/fn-recover-return-sign.rs:25:24
21+
|
22+
LL | let bar = |a: bool|: bool { a };
23+
| ^ help: use `->` instead
24+
25+
error: aborting due to 4 previous errors
26+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Separate test file because `Fn() => bool` isn't getting fixed and rustfix complained that
2+
// even though a fix was applied the code was still incorrect
3+
4+
fn foo() => impl Fn() => bool {
5+
//~^ ERROR return types are denoted using `->`
6+
//~| ERROR expected one of `+`, `->`, `::`, `;`, `where`, or `{`, found `=>`
7+
unimplemented!()
8+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: return types are denoted using `->`
2+
--> $DIR/fn-recover-return-sign2.rs:4:10
3+
|
4+
LL | fn foo() => impl Fn() => bool {
5+
| ^^ help: use `->` instead
6+
7+
error: expected one of `+`, `->`, `::`, `;`, `where`, or `{`, found `=>`
8+
--> $DIR/fn-recover-return-sign2.rs:4:23
9+
|
10+
LL | fn foo() => impl Fn() => bool {
11+
| ^^ expected one of `+`, `->`, `::`, `;`, `where`, or `{`
12+
13+
error: aborting due to 2 previous errors
14+

src/test/ui/parser/fn-colon-return-type.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
fn foo(x: i32): i32 { //~ ERROR expected one of `->`, `;`, `where`, or `{`, found `:`
1+
fn foo(x: i32): i32 {
2+
//~^ ERROR return types are denoted using `->`
23
x
34
}
45

Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: expected one of `->`, `;`, `where`, or `{`, found `:`
1+
error: return types are denoted using `->`
22
--> $DIR/fn-colon-return-type.rs:1:15
33
|
44
LL | fn foo(x: i32): i32 {
5-
| ^ expected one of `->`, `;`, `where`, or `{`
5+
| ^ help: use `->` instead
66

77
error: aborting due to previous error
88

0 commit comments

Comments
 (0)