Skip to content

Commit ed60cf2

Browse files
estebankestebank
committed
When encountering chained operators use heuristics to recover from bad turbofish
1 parent f2023ac commit ed60cf2

File tree

5 files changed

+121
-27
lines changed

5 files changed

+121
-27
lines changed

‎src/libsyntax/parse/diagnostics.rs

Lines changed: 88 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -543,16 +543,25 @@ impl<'a> Parser<'a> {
543543
}
544544

545545
/// Produces an error if comparison operators are chained (RFC #558).
546-
/// We only need to check the LHS, not the RHS, because all comparison ops
547-
/// have same precedence and are left-associative.
548-
crate fn check_no_chained_comparison(&self, lhs: &Expr, outer_op: &AssocOp) -> PResult<'a, ()> {
549-
debug_assert!(outer_op.is_comparison(),
550-
"check_no_chained_comparison: {:?} is not comparison",
551-
outer_op);
546+
/// We only need to check the LHS, not the RHS, because all comparison ops have same
547+
/// precedence and are left-associative.
548+
///
549+
/// This can also be hit if someone incorrectly writes `foo<bar>()` when they should have used
550+
/// the turbofish syntax. We attempt some heuristic recovery if that is the case.
551+
crate fn check_no_chained_comparison(
552+
&mut self,
553+
lhs: &Expr,
554+
outer_op: &AssocOp,
555+
) -> PResult<'a, Option<P<Expr>>> {
556+
debug_assert!(
557+
outer_op.is_comparison(),
558+
"check_no_chained_comparison: {:?} is not comparison",
559+
outer_op,
560+
);
552561
match lhs.kind {
553562
ExprKind::Binary(op, _, _) if op.node.is_comparison() => {
554563
// Respan to include both operators.
555-
let op_span = op.span.to(self.token.span);
564+
let op_span = op.span.to(self.prev_span);
556565
let mut err = self.struct_span_err(
557566
op_span,
558567
"chained comparison operators require parentheses",
@@ -561,17 +570,84 @@ impl<'a> Parser<'a> {
561570
*outer_op == AssocOp::Less || // Include `<` to provide this recommendation
562571
*outer_op == AssocOp::Greater // even in a case like the following:
563572
{ // Foo<Bar<Baz<Qux, ()>>>
564-
err.help(
565-
"use `::<...>` instead of `<...>` if you meant to specify type arguments");
566-
err.help("or use `(...)` if you meant to specify fn arguments");
567-
// These cases cause too many knock-down errors, bail out (#61329).
573+
let msg = "use `::<...>` instead of `<...>` if you meant to specify type \
574+
arguments";
575+
if *outer_op == AssocOp::Less {
576+
// if self.look_ahead(1, |t| t.kind == token::Lt || t.kind == token::ModSep) {
577+
let snapshot = self.clone();
578+
self.bump();
579+
// So far we have parsed `foo<bar<`
580+
let mut acc = 1;
581+
while acc > 0 {
582+
match &self.token.kind {
583+
token::Lt => {
584+
acc += 1;
585+
}
586+
token::Gt => {
587+
acc -= 1;
588+
}
589+
token::BinOp(token::Shr) => {
590+
acc -= 2;
591+
}
592+
token::Eof => {
593+
break;
594+
}
595+
_ => {}
596+
}
597+
self.bump();
598+
}
599+
if self.token.kind != token::OpenDelim(token::Paren) {
600+
mem::replace(self, snapshot.clone());
601+
}
602+
}
603+
if self.token.kind == token::OpenDelim(token::Paren) {
604+
err.span_suggestion(
605+
op_span.shrink_to_lo(),
606+
msg,
607+
"::".to_string(),
608+
Applicability::MaybeIncorrect,
609+
);
610+
let snapshot = self.clone();
611+
self.bump();
612+
let mut acc = 1;
613+
while acc > 0 {
614+
match &self.token.kind {
615+
token::OpenDelim(token::Paren) => {
616+
acc += 1;
617+
}
618+
token::CloseDelim(token::Paren) => {
619+
acc -= 1;
620+
}
621+
token::Eof => {
622+
break;
623+
}
624+
_ => {}
625+
}
626+
self.bump();
627+
}
628+
if self.token.kind == token::Eof {
629+
mem::replace(self, snapshot);
630+
return Err(err);
631+
} else {
632+
err.emit();
633+
return Ok(Some(self.mk_expr(
634+
lhs.span.to(self.prev_span),
635+
ExprKind::Err,
636+
ThinVec::new(),
637+
)));
638+
}
639+
} else {
640+
err.help(msg);
641+
err.help("or use `(...)` if you meant to specify fn arguments");
642+
// These cases cause too many knock-down errors, bail out (#61329).
643+
}
568644
return Err(err);
569645
}
570646
err.emit();
571647
}
572648
_ => {}
573649
}
574-
Ok(())
650+
Ok(None)
575651
}
576652

577653
crate fn maybe_report_ambiguous_plus(

‎src/libsyntax/parse/parser/expr.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,9 @@ impl<'a> Parser<'a> {
238238

239239
self.bump();
240240
if op.is_comparison() {
241-
self.check_no_chained_comparison(&lhs, &op)?;
241+
if let Some(expr) = self.check_no_chained_comparison(&lhs, &op)? {
242+
return Ok(expr);
243+
}
242244
}
243245
// Special cases:
244246
if op == AssocOp::As {

‎src/test/ui/did_you_mean/issue-40396.stderr

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,17 @@ error: chained comparison operators require parentheses
22
--> $DIR/issue-40396.rs:2:20
33
|
44
LL | (0..13).collect<Vec<i32>>();
5-
| ^^^^^^^^
5+
| ^^^^^
6+
help: use `::<...>` instead of `<...>` if you meant to specify type arguments
67
|
7-
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
8-
= help: or use `(...)` if you meant to specify fn arguments
8+
LL | (0..13).collect::<Vec<i32>>();
9+
| ^^
910

1011
error: chained comparison operators require parentheses
1112
--> $DIR/issue-40396.rs:7:8
1213
|
1314
LL | Vec<i32>::new();
14-
| ^^^^^^^
15+
| ^^^^^
1516
|
1617
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
1718
= help: or use `(...)` if you meant to specify fn arguments
@@ -20,10 +21,11 @@ error: chained comparison operators require parentheses
2021
--> $DIR/issue-40396.rs:12:20
2122
|
2223
LL | (0..13).collect<Vec<i32>();
23-
| ^^^^^^^^
24+
| ^^^^^
25+
help: use `::<...>` instead of `<...>` if you meant to specify type arguments
2426
|
25-
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
26-
= help: or use `(...)` if you meant to specify fn arguments
27+
LL | (0..13).collect::<Vec<i32>();
28+
| ^^
2729

2830
error: aborting due to 3 previous errors
2931

‎src/test/ui/parser/require-parens-for-chained-comparison.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,8 @@ fn main() {
1313
f<X>();
1414
//~^ ERROR chained comparison operators require parentheses
1515
//~| HELP: use `::<...>` instead of `<...>`
16-
//~| HELP: or use `(...)`
16+
17+
f<Result<Option<X>, Option<Option<X>>>(1, 2);
18+
//~^ ERROR chained comparison operators require parentheses
19+
//~| HELP: use `::<...>` instead of `<...>`
1720
}

‎src/test/ui/parser/require-parens-for-chained-comparison.stderr

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,33 @@ error: chained comparison operators require parentheses
22
--> $DIR/require-parens-for-chained-comparison.rs:5:11
33
|
44
LL | false == false == false;
5-
| ^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^
66

77
error: chained comparison operators require parentheses
88
--> $DIR/require-parens-for-chained-comparison.rs:8:11
99
|
1010
LL | false == 0 < 2;
11-
| ^^^^^^^^
11+
| ^^^^^^
1212

1313
error: chained comparison operators require parentheses
1414
--> $DIR/require-parens-for-chained-comparison.rs:13:6
1515
|
1616
LL | f<X>();
17-
| ^^^^
17+
| ^^^
18+
help: use `::<...>` instead of `<...>` if you meant to specify type arguments
1819
|
19-
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
20-
= help: or use `(...)` if you meant to specify fn arguments
20+
LL | f::<X>();
21+
| ^^
22+
23+
error: chained comparison operators require parentheses
24+
--> $DIR/require-parens-for-chained-comparison.rs:17:6
25+
|
26+
LL | f<Result<Option<X>, Option<Option<X>>>(1, 2);
27+
| ^^^^^^^^
28+
help: use `::<...>` instead of `<...>` if you meant to specify type arguments
29+
|
30+
LL | f::<Result<Option<X>, Option<Option<X>>>(1, 2);
31+
| ^^
2132

2233
error[E0308]: mismatched types
2334
--> $DIR/require-parens-for-chained-comparison.rs:8:14
@@ -37,6 +48,6 @@ LL | false == 0 < 2;
3748
= note: expected type `bool`
3849
found type `{integer}`
3950

40-
error: aborting due to 5 previous errors
51+
error: aborting due to 6 previous errors
4152

4253
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)