Skip to content

Commit

Permalink
Rollup merge of rust-lang#64192 - estebank:turbofish-madness, r=petro…
Browse files Browse the repository at this point in the history
…chenkov

Bail out when encountering likely missing turbofish in parser

When encountering a likely intended turbofish without `::`, bubble
up the diagnostic instead of emitting it to allow the parser to recover
more gracefully and avoid uneccessary type errors that are likely to be
wrong.

Fix rust-lang#61329.
  • Loading branch information
Centril authored Sep 7, 2019
2 parents da61325 + dc613c6 commit fae7bc7
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 138 deletions.
5 changes: 4 additions & 1 deletion src/libsyntax/parse/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ impl<'a> Parser<'a> {
/// Produce an error if comparison operators are chained (RFC #558).
/// We only need to check lhs, not rhs, because all comparison ops
/// have same precedence and are left-associative
crate fn check_no_chained_comparison(&self, lhs: &Expr, outer_op: &AssocOp) {
crate fn check_no_chained_comparison(&self, lhs: &Expr, outer_op: &AssocOp) -> PResult<'a, ()> {
debug_assert!(outer_op.is_comparison(),
"check_no_chained_comparison: {:?} is not comparison",
outer_op);
Expand All @@ -563,11 +563,14 @@ impl<'a> Parser<'a> {
err.help(
"use `::<...>` instead of `<...>` if you meant to specify type arguments");
err.help("or use `(...)` if you meant to specify fn arguments");
// These cases cause too many knock-down errors, bail out (#61329).
return Err(err);
}
err.emit();
}
_ => {}
}
Ok(())
}

crate fn maybe_report_ambiguous_plus(
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ impl<'a> Parser<'a> {

self.bump();
if op.is_comparison() {
self.check_no_chained_comparison(&lhs, &op);
self.check_no_chained_comparison(&lhs, &op)?;
}
// Special cases:
if op == AssocOp::As {
Expand Down
47 changes: 25 additions & 22 deletions src/test/ui-fulldeps/pprust-expr-roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ use syntax::print::pprust;
use syntax::ptr::P;


fn parse_expr(ps: &ParseSess, src: &str) -> P<Expr> {
fn parse_expr(ps: &ParseSess, src: &str) -> Option<P<Expr>> {
let src_as_string = src.to_string();

let mut p = parse::new_parser_from_source_str(ps,
FileName::Custom(src_as_string.clone()),
src_as_string);
p.parse_expr().unwrap()
let mut p = parse::new_parser_from_source_str(
ps,
FileName::Custom(src_as_string.clone()),
src_as_string,
);
p.parse_expr().map_err(|mut e| e.cancel()).ok()
}


Expand Down Expand Up @@ -209,22 +211,23 @@ fn run() {
let printed = pprust::expr_to_string(&e);
println!("printed: {}", printed);

let mut parsed = parse_expr(&ps, &printed);

// We want to know if `parsed` is structurally identical to `e`, ignoring trivial
// differences like placement of `Paren`s or the exact ranges of node spans.
// Unfortunately, there is no easy way to make this comparison. Instead, we add `Paren`s
// everywhere we can, then pretty-print. This should give an unambiguous representation of
// each `Expr`, and it bypasses nearly all of the parenthesization logic, so we aren't
// relying on the correctness of the very thing we're testing.
RemoveParens.visit_expr(&mut e);
AddParens.visit_expr(&mut e);
let text1 = pprust::expr_to_string(&e);
RemoveParens.visit_expr(&mut parsed);
AddParens.visit_expr(&mut parsed);
let text2 = pprust::expr_to_string(&parsed);
assert!(text1 == text2,
"exprs are not equal:\n e = {:?}\n parsed = {:?}",
text1, text2);
// Ignore expressions with chained comparisons that fail to parse
if let Some(mut parsed) = parse_expr(&ps, &printed) {
// We want to know if `parsed` is structurally identical to `e`, ignoring trivial
// differences like placement of `Paren`s or the exact ranges of node spans.
// Unfortunately, there is no easy way to make this comparison. Instead, we add `Paren`s
// everywhere we can, then pretty-print. This should give an unambiguous representation
// of each `Expr`, and it bypasses nearly all of the parenthesization logic, so we
// aren't relying on the correctness of the very thing we're testing.
RemoveParens.visit_expr(&mut e);
AddParens.visit_expr(&mut e);
let text1 = pprust::expr_to_string(&e);
RemoveParens.visit_expr(&mut parsed);
AddParens.visit_expr(&mut parsed);
let text2 = pprust::expr_to_string(&parsed);
assert!(text1 == text2,
"exprs are not equal:\n e = {:?}\n parsed = {:?}",
text1, text2);
}
});
}
11 changes: 0 additions & 11 deletions src/test/ui/did_you_mean/issue-40396.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,16 @@
fn foo() {
(0..13).collect<Vec<i32>>();
//~^ ERROR chained comparison
//~| ERROR expected value, found struct `Vec`
//~| ERROR expected value, found builtin type `i32`
//~| ERROR attempted to take value of method `collect`
}

fn bar() {
Vec<i32>::new();
//~^ ERROR chained comparison
//~| ERROR expected value, found struct `Vec`
//~| ERROR expected value, found builtin type `i32`
//~| ERROR cannot find function `new` in the crate root
}

fn qux() {
(0..13).collect<Vec<i32>();
//~^ ERROR chained comparison
//~| ERROR chained comparison
//~| ERROR expected value, found struct `Vec`
//~| ERROR expected value, found builtin type `i32`
//~| ERROR attempted to take value of method `collect`
//~| ERROR mismatched types
}

fn main() {}
80 changes: 3 additions & 77 deletions src/test/ui/did_you_mean/issue-40396.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | (0..13).collect<Vec<i32>>();
= help: or use `(...)` if you meant to specify fn arguments

error: chained comparison operators require parentheses
--> $DIR/issue-40396.rs:10:8
--> $DIR/issue-40396.rs:7:8
|
LL | Vec<i32>::new();
| ^^^^^^^
Expand All @@ -17,87 +17,13 @@ LL | Vec<i32>::new();
= help: or use `(...)` if you meant to specify fn arguments

error: chained comparison operators require parentheses
--> $DIR/issue-40396.rs:18:20
--> $DIR/issue-40396.rs:12:20
|
LL | (0..13).collect<Vec<i32>();
| ^^^^^^^^
|
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments

error: chained comparison operators require parentheses
--> $DIR/issue-40396.rs:18:24
|
LL | (0..13).collect<Vec<i32>();
| ^^^^^^
|
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments

error[E0423]: expected value, found struct `Vec`
--> $DIR/issue-40396.rs:2:21
|
LL | (0..13).collect<Vec<i32>>();
| ^^^ did you mean `Vec { /* fields */ }`?

error[E0423]: expected value, found builtin type `i32`
--> $DIR/issue-40396.rs:2:25
|
LL | (0..13).collect<Vec<i32>>();
| ^^^ not a value

error[E0423]: expected value, found struct `Vec`
--> $DIR/issue-40396.rs:10:5
|
LL | Vec<i32>::new();
| ^^^ did you mean `Vec { /* fields */ }`?

error[E0423]: expected value, found builtin type `i32`
--> $DIR/issue-40396.rs:10:9
|
LL | Vec<i32>::new();
| ^^^ not a value

error[E0425]: cannot find function `new` in the crate root
--> $DIR/issue-40396.rs:10:15
|
LL | Vec<i32>::new();
| ^^^ not found in the crate root

error[E0423]: expected value, found struct `Vec`
--> $DIR/issue-40396.rs:18:21
|
LL | (0..13).collect<Vec<i32>();
| ^^^ did you mean `Vec { /* fields */ }`?

error[E0423]: expected value, found builtin type `i32`
--> $DIR/issue-40396.rs:18:25
|
LL | (0..13).collect<Vec<i32>();
| ^^^ not a value

error[E0615]: attempted to take value of method `collect` on type `std::ops::Range<{integer}>`
--> $DIR/issue-40396.rs:2:13
|
LL | (0..13).collect<Vec<i32>>();
| ^^^^^^^ help: use parentheses to call the method: `collect()`

error[E0615]: attempted to take value of method `collect` on type `std::ops::Range<{integer}>`
--> $DIR/issue-40396.rs:18:13
|
LL | (0..13).collect<Vec<i32>();
| ^^^^^^^ help: use parentheses to call the method: `collect()`

error[E0308]: mismatched types
--> $DIR/issue-40396.rs:18:29
|
LL | (0..13).collect<Vec<i32>();
| ^^ expected bool, found ()
|
= note: expected type `bool`
found type `()`

error: aborting due to 14 previous errors
error: aborting due to 3 previous errors

Some errors have detailed explanations: E0308, E0423, E0425, E0615.
For more information about an error, try `rustc --explain E0308`.
3 changes: 1 addition & 2 deletions src/test/ui/parser/require-parens-for-chained-comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ fn main() {
//~| ERROR: mismatched types

f<X>();
//~^ ERROR: chained comparison operators require parentheses
//~| ERROR: binary operation `<` cannot be applied to type `fn() {f::<_>}`
//~^ ERROR chained comparison operators require parentheses
//~| HELP: use `::<...>` instead of `<...>`
//~| HELP: or use `(...)`
}
15 changes: 2 additions & 13 deletions src/test/ui/parser/require-parens-for-chained-comparison.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,6 @@ LL | false == 0 < 2;
= note: expected type `bool`
found type `{integer}`

error[E0369]: binary operation `<` cannot be applied to type `fn() {f::<_>}`
--> $DIR/require-parens-for-chained-comparison.rs:13:6
|
LL | f<X>();
| -^- X
| |
| fn() {f::<_>}
|
= note: an implementation of `std::cmp::PartialOrd` might be missing for `fn() {f::<_>}`

error: aborting due to 6 previous errors
error: aborting due to 5 previous errors

Some errors have detailed explanations: E0308, E0369.
For more information about an error, try `rustc --explain E0308`.
For more information about this error, try `rustc --explain E0308`.
1 change: 0 additions & 1 deletion src/test/ui/parser/trait-object-lifetime-parens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ fn check<'a>() {
let _: Box<('a) + Trait>;
//~^ ERROR expected type, found `'a`
//~| ERROR expected `:`, found `)`
//~| ERROR chained comparison operators require parentheses
}

fn main() {}
11 changes: 1 addition & 10 deletions src/test/ui/parser/trait-object-lifetime-parens.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,6 @@ error: expected `:`, found `)`
LL | let _: Box<('a) + Trait>;
| ^ expected `:`

error: chained comparison operators require parentheses
--> $DIR/trait-object-lifetime-parens.rs:9:15
|
LL | let _: Box<('a) + Trait>;
| ^^^^^^^^^^^^^^^
|
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments

error: expected type, found `'a`
--> $DIR/trait-object-lifetime-parens.rs:9:17
|
Expand All @@ -33,5 +24,5 @@ LL | let _: Box<('a) + Trait>;
| |
| while parsing the type for `_`

error: aborting due to 5 previous errors
error: aborting due to 4 previous errors

0 comments on commit fae7bc7

Please sign in to comment.