Skip to content

Commit

Permalink
Rollup merge of #70209 - Centril:recover-quant-closure, r=petrochenkov
Browse files Browse the repository at this point in the history
parser: recover on `for<'a> |...| body` closures

When encountering `for` and `<` is 1 token ahead, interpret this as an explicitly quantified generic closure and recover, rather than attempting to parse a `for` loop. This provides both improved diagnostics as well as an insurance policy for the ability to use this as the syntax for generic closures in the future.

As requested by r? @eddyb
  • Loading branch information
Dylan-DPC authored Mar 22, 2020
2 parents 9890d9a + 4d30b92 commit ea44d71
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 14 deletions.
33 changes: 31 additions & 2 deletions src/librustc_parse/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,17 @@ impl<'a> Parser<'a> {
self.parse_closure_expr(attrs)
} else if self.eat_keyword(kw::If) {
self.parse_if_expr(attrs)
} else if self.eat_keyword(kw::For) {
self.parse_for_expr(None, self.prev_token.span, attrs)
} else if self.check_keyword(kw::For) {
if self.choose_generics_over_qpath(1) {
// NOTE(Centril, eddyb): DO NOT REMOVE! Beyond providing parser recovery,
// this is an insurance policy in case we allow qpaths in (tuple-)struct patterns.
// When `for <Foo as Bar>::Proj in $expr $block` is wanted,
// you can disambiguate in favor of a pattern with `(...)`.
self.recover_quantified_closure_expr(attrs)
} else {
assert!(self.eat_keyword(kw::For));
self.parse_for_expr(None, self.prev_token.span, attrs)
}
} else if self.eat_keyword(kw::While) {
self.parse_while_expr(None, self.prev_token.span, attrs)
} else if let Some(label) = self.eat_label() {
Expand Down Expand Up @@ -1417,6 +1426,26 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr(blk.span, ExprKind::Block(blk, opt_label), attrs))
}

/// Recover on an explicitly quantified closure expression, e.g., `for<'a> |x: &'a u8| *x + 1`.
fn recover_quantified_closure_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
let _ = self.parse_late_bound_lifetime_defs()?;
let span_for = lo.to(self.prev_token.span);
let closure = self.parse_closure_expr(attrs)?;

self.struct_span_err(span_for, "cannot introduce explicit parameters for a closure")
.span_label(closure.span, "the parameters are attached to this closure")
.span_suggestion(
span_for,
"remove the parameters",
String::new(),
Applicability::MachineApplicable,
)
.emit();

Ok(self.mk_expr_err(lo.to(closure.span)))
}

/// Parses a closure expression (e.g., `move |args| expr`).
fn parse_closure_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
Expand Down
19 changes: 8 additions & 11 deletions src/librustc_parse/parser/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<'a> Parser<'a> {
// We are considering adding generics to the `where` keyword as an alternative higher-rank
// parameter syntax (as in `where<'a>` or `where<T>`. To avoid that being a breaking
// change we parse those generics now, but report an error.
if self.choose_generics_over_qpath() {
if self.choose_generics_over_qpath(0) {
let generics = self.parse_generics()?;
self.struct_span_err(
generics.span,
Expand Down Expand Up @@ -257,7 +257,7 @@ impl<'a> Parser<'a> {
}
}

pub(super) fn choose_generics_over_qpath(&self) -> bool {
pub(super) fn choose_generics_over_qpath(&self, start: usize) -> bool {
// There's an ambiguity between generic parameters and qualified paths in impls.
// If we see `<` it may start both, so we have to inspect some following tokens.
// The following combinations can only start generics,
Expand All @@ -274,15 +274,12 @@ impl<'a> Parser<'a> {
// we disambiguate it in favor of generics (`impl<T> ::absolute::Path<T> { ... }`)
// because this is what almost always expected in practice, qualified paths in impls
// (`impl <Type>::AssocTy { ... }`) aren't even allowed by type checker at the moment.
self.token == token::Lt
&& (self.look_ahead(1, |t| t == &token::Pound || t == &token::Gt)
|| self.look_ahead(1, |t| t.is_lifetime() || t.is_ident())
&& self.look_ahead(2, |t| {
t == &token::Gt
|| t == &token::Comma
|| t == &token::Colon
|| t == &token::Eq
self.look_ahead(start, |t| t == &token::Lt)
&& (self.look_ahead(start + 1, |t| t == &token::Pound || t == &token::Gt)
|| self.look_ahead(start + 1, |t| t.is_lifetime() || t.is_ident())
&& self.look_ahead(start + 2, |t| {
matches!(t.kind, token::Gt | token::Comma | token::Colon | token::Eq)
})
|| self.is_keyword_ahead(1, &[kw::Const]))
|| self.is_keyword_ahead(start + 1, &[kw::Const]))
}
}
2 changes: 1 addition & 1 deletion src/librustc_parse/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ impl<'a> Parser<'a> {
self.expect_keyword(kw::Impl)?;

// First, parse generic parameters if necessary.
let mut generics = if self.choose_generics_over_qpath() {
let mut generics = if self.choose_generics_over_qpath(0) {
self.parse_generics()?
} else {
let mut generics = Generics::default();
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/parser/recover-quantified-closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn main() {
for<'a> |x: &'a u8| *x + 1;
//~^ ERROR cannot introduce explicit parameters for a closure
}

enum Foo { Bar }
fn foo(x: impl Iterator<Item = Foo>) {
for <Foo>::Bar in x {}
//~^ ERROR expected one of `move`, `static`, `|`
}
16 changes: 16 additions & 0 deletions src/test/ui/parser/recover-quantified-closure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: cannot introduce explicit parameters for a closure
--> $DIR/recover-quantified-closure.rs:2:5
|
LL | for<'a> |x: &'a u8| *x + 1;
| ^^^^^^^ ------------------ the parameters are attached to this closure
| |
| help: remove the parameters

error: expected one of `move`, `static`, `|`, or `||`, found `::`
--> $DIR/recover-quantified-closure.rs:8:14
|
LL | for <Foo>::Bar in x {}
| ^^ expected one of `move`, `static`, `|`, or `||`

error: aborting due to 2 previous errors

0 comments on commit ea44d71

Please sign in to comment.