bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #100011 - compiler-errors:let-chain-restriction, r=fee1-dead

Browse source 
Use Parser's `restrictions` instead of `let_expr_allowed`

This also means that the `ALLOW_LET` flag is reset properly for subexpressions, so we can properly deny things like `a && (b && let c = d)`. Also the parser is a tiny bit smaller now.

It doesn't reject _all_ bad `let` expr usages, just a bit more.

cc `@c410-f3r`
This commit is contained in:
Matthias Krüger 2022-08-02 07:30:44 +02:00 • committed by GitHub
commit beb4cdddde
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 365 additions and 306 deletions
Download patch file Download diff file Expand all files Collapse all files

60
compiler/rustc_parse/src/parser/expr.rs
View file

@ -1391,8 +1391,6 @@ impl<'a> Parser<'a> {
} else if self.is_do_yeet() {
self.parse_yeet_expr(attrs)
} else if self.check_keyword(kw::Let) {
self.manage_let_chains_context();
self.bump();
self.parse_let_expr(attrs)
} else if self.eat_keyword(kw::Underscore) {
Ok(self.mk_expr(self.prev_token.span, ExprKind::Underscore, attrs))
@ -2342,32 +2340,24 @@ impl<'a> Parser<'a> {
/// Parses the condition of a `if` or `while` expression.
fn parse_cond_expr(&mut self) -> PResult<'a, P<Expr>> {
self.with_let_management(true, |local_self| {
local_self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)
})
}
// Checks if `let` is in an invalid position like `let x = let y = 1;` or
// if the current `let` is in a let_chains context but nested in another
// expression like `if let Some(_) = _opt && [1, 2, 3][let _ = ()] = 1`.
//
// This method expects that the current token is `let`.
fn manage_let_chains_context(&mut self) {
debug_assert!(matches!(self.token.kind, TokenKind::Ident(kw::Let, _)));
let is_in_a_let_chains_context_but_nested_in_other_expr = self.let_expr_allowed
&& !matches!(
self.prev_token.kind,
TokenKind::AndAnd | TokenKind::Ident(kw::If, _) | TokenKind::Ident(kw::While, _)
);
if !self.let_expr_allowed || is_in_a_let_chains_context_but_nested_in_other_expr {
self.struct_span_err(self.token.span, "expected expression, found `let` statement")
.emit();
}
self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL | Restrictions::ALLOW_LET, None)
}
/// Parses a `let $pat = $expr` pseudo-expression.
/// The `let` token has already been eaten.
fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
// This is a *approximate* heuristic that detects if `let` chains are
// being parsed in the right position. It's approximate because it
// doesn't deny all invalid `let` expressions, just completely wrong usages.
let not_in_chain = !matches!(
self.prev_token.kind,
TokenKind::AndAnd | TokenKind::Ident(kw::If, _) | TokenKind::Ident(kw::While, _)
);
if !self.restrictions.contains(Restrictions::ALLOW_LET) || not_in_chain {
self.struct_span_err(self.token.span, "expected expression, found `let` statement")
.emit();
}
self.bump(); // Eat `let` token
let lo = self.prev_token.span;
let pat = self.parse_pat_allow_top_alt(
None,
@ -2687,7 +2677,9 @@ impl<'a> Parser<'a> {
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> bool {
match expr.kind {
ExprKind::Binary(_, ref lhs, ref rhs) => check_let_expr(lhs) || check_let_expr(rhs),
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, ref lhs, ref rhs) => {
check_let_expr(lhs) || check_let_expr(rhs)
}
ExprKind::Let(..) => true,
_ => false,
}
@ -2703,9 +2695,8 @@ impl<'a> Parser<'a> {
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let cond = this.with_let_management(true, |local_this| local_this.parse_expr())?;
let has_let_expr = check_let_expr(&cond);
if has_let_expr {
let cond = this.parse_expr_res(Restrictions::ALLOW_LET, None)?;
if check_let_expr(&cond) {
let span = if_span.to(cond.span);
this.sess.gated_spans.gate(sym::if_let_guard, span);
}
@ -3279,17 +3270,4 @@ impl<'a> Parser<'a> {
Ok((res, trailing))
})
}
// Calls `f` with the internal `let_expr_allowed` set to `let_expr_allowed` and then
// sets the internal `let_expr_allowed` back to its original value.
fn with_let_management<T>(
&mut self,
let_expr_allowed: bool,
f: impl FnOnce(&mut Self) -> T,
) -> T {
let last_let_expr_allowed = mem::replace(&mut self.let_expr_allowed, let_expr_allowed);
let rslt = f(self);
self.let_expr_allowed = last_let_expr_allowed;
rslt
}
}

7
compiler/rustc_parse/src/parser/mod.rs
View file

@ -47,6 +47,7 @@ bitflags::bitflags! {
const STMT_EXPR = 1 << 0;
const NO_STRUCT_LITERAL = 1 << 1;
const CONST_EXPR = 1 << 2;
const ALLOW_LET = 1 << 3;
}
}
@ -147,15 +148,12 @@ pub struct Parser<'a> {
/// This allows us to recover when the user forget to add braces around
/// multiple statements in the closure body.
pub current_closure: Option<ClosureSpans>,
/// Used to track where `let`s are allowed. For example, `if true && let 1 = 1` is valid
/// but `[1, 2, 3][let _ = ()]` is not.
let_expr_allowed: bool,
}
// This type is used a lot, e.g. it's cloned when matching many declarative macro rules. Make sure
// it doesn't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Parser<'_>, 336);
rustc_data_structures::static_assert_size!(Parser<'_>, 328);
/// Stores span information about a closure.
#[derive(Clone)]
@ -462,7 +460,6 @@ impl<'a> Parser<'a> {
inner_attr_ranges: Default::default(),
},
current_closure: None,
let_expr_allowed: false,
};
// Make parser point to the first token.