Skip to content
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

Bail out when encountering likely missing turbofish in parser #64192

Merged
merged 2 commits into from
Sep 7, 2019
Merged
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
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