Skip to content

Commit

Permalink
Rollup merge of #95008 - c410-f3r:let-chains-paren, r=wesleywiser
Browse files Browse the repository at this point in the history 
[`let_chains`] Forbid `let` inside parentheses

Parenthesizes are mostly a no-op in let chains, in other words, they are mostly ignored.

```rust
let opt = Some(Some(1i32));

if (let Some(a) = opt && (let Some(b) = a)) && b == 1 {
    println!("`b` is declared inside but used outside");
}
```

As seen above, such behavior can lead to confusion.

A proper fix or nested encapsulation would probably require research, time and a modified MIR graph so in this PR I simply denied any `let` inside parentheses. Non-let stuff are still allowed.

```rust
fn main() {
    let fun = || true;

    if let true = (true && fun()) && (true) {
        println!("Allowed");
    }
}
```

It is worth noting that `let ...`  is not an expression and the RFC did not mention this specific situation.

cc `@matthewjasper`
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Apr 11, 2022
2 parents 625e4dd + 6ee3c47 commit 2ad701e
Show file tree
Hide file tree
Showing 6 changed files with 751 additions and 473 deletions.
54 changes: 42 additions & 12 deletions compiler/rustc_ast_passes/src/ast_validation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,21 @@ impl<'a> AstValidator<'a> {
let err = "`let` expressions are not supported here";
let mut diag = sess.struct_span_err(expr.span, err);
diag.note("only supported directly in conditions of `if` and `while` expressions");
diag.note("as well as when nested within `&&` and parentheses in those conditions");
if let ForbiddenLetReason::ForbiddenWithOr(span) = forbidden_let_reason {
diag.span_note(
span,
"`||` operators are not currently supported in let chain expressions",
);
match forbidden_let_reason {
ForbiddenLetReason::GenericForbidden => {}
ForbiddenLetReason::NotSupportedOr(span) => {
diag.span_note(
span,
"`||` operators are not supported in let chain expressions",
);
}
ForbiddenLetReason::NotSupportedParentheses(span) => {
diag.span_note(
span,
"`let`s wrapped in parentheses are not supported in a context with let \
chains",
);
}
}
diag.emit();
} else {
Expand Down Expand Up @@ -1009,9 +1018,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.with_let_management(Some(ForbiddenLetReason::GenericForbidden), |this, forbidden_let_reason| {
match &expr.kind {
ExprKind::Binary(Spanned { node: BinOpKind::Or, span }, lhs, rhs) => {
let forbidden_let_reason = Some(ForbiddenLetReason::ForbiddenWithOr(*span));
this.with_let_management(forbidden_let_reason, |this, _| this.visit_expr(lhs));
this.with_let_management(forbidden_let_reason, |this, _| this.visit_expr(rhs));
let local_reason = Some(ForbiddenLetReason::NotSupportedOr(*span));
this.with_let_management(local_reason, |this, _| this.visit_expr(lhs));
this.with_let_management(local_reason, |this, _| this.visit_expr(rhs));
}
ExprKind::If(cond, then, opt_else) => {
this.visit_block(then);
Expand All @@ -1036,7 +1045,23 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}
}
}
ExprKind::Paren(_) | ExprKind::Binary(Spanned { node: BinOpKind::And, .. }, ..) => {
ExprKind::Paren(local_expr) => {
fn has_let_expr(expr: &Expr) -> bool {
match expr.kind {
ExprKind::Binary(_, ref lhs, ref rhs) => has_let_expr(lhs) || has_let_expr(rhs),
ExprKind::Let(..) => true,
_ => false,
}
}
let local_reason = if has_let_expr(local_expr) {
Some(ForbiddenLetReason::NotSupportedParentheses(local_expr.span))
}
else {
forbidden_let_reason
};
this.with_let_management(local_reason, |this, _| this.visit_expr(local_expr));
}
ExprKind::Binary(Spanned { node: BinOpKind::And, .. }, ..) => {
this.with_let_management(forbidden_let_reason, |this, _| visit::walk_expr(this, expr));
return;
}
Expand Down Expand Up @@ -1810,8 +1835,13 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) ->
/// Used to forbid `let` expressions in certain syntactic locations.
#[derive(Clone, Copy)]
enum ForbiddenLetReason {
/// A let chain with the `||` operator
ForbiddenWithOr(Span),
/// `let` is not valid and the source environment is not important
GenericForbidden,
/// A let chain with the `||` operator
NotSupportedOr(Span),
/// A let chain with invalid parentheses
///
/// For exemple, `let 1 = 1 && (expr && expr)` is allowed
/// but `(let 1 = 1 && (let 1 = 1 && (let 1 = 1))) && let a = 1` is not
NotSupportedParentheses(Span),
}
102 changes: 102 additions & 0 deletions src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,67 @@ use std::ops::Range;

fn main() {}

fn _if() {
if (let 0 = 1) {}
//~^ ERROR `let` expressions are not supported here

if (((let 0 = 1))) {}
//~^ ERROR `let` expressions are not supported here

if (let 0 = 1) && true {}
//~^ ERROR `let` expressions are not supported here

if true && (let 0 = 1) {}
//~^ ERROR `let` expressions are not supported here

if (let 0 = 1) && (let 0 = 1) {}
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here

if let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) {}
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
}

fn _while() {
while (let 0 = 1) {}
//~^ ERROR `let` expressions are not supported here

while (((let 0 = 1))) {}
//~^ ERROR `let` expressions are not supported here

while (let 0 = 1) && true {}
//~^ ERROR `let` expressions are not supported here

while true && (let 0 = 1) {}
//~^ ERROR `let` expressions are not supported here

while (let 0 = 1) && (let 0 = 1) {}
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here

while let 0 = 1 && let 1 = 2 && (let 2 = 3 && let 3 = 4 && let 4 = 5) {}
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
}

fn _macros() {
macro_rules! use_expr {
($e:expr) => {
if $e {}
while $e {}
}
}
use_expr!((let 0 = 1 && 0 == 0));
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
use_expr!((let 0 = 1));
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
}

fn nested_within_if_expr() {
if &let 0 = 0 {} //~ ERROR `let` expressions are not supported here
//~^ ERROR mismatched types
Expand Down Expand Up @@ -234,3 +295,44 @@ fn inside_const_generic_arguments() {
//~| ERROR expressions must be enclosed in braces
>::O == 5 {}
}

fn with_parenthesis() {
let opt = Some(Some(1i32));

if (let Some(a) = opt && true) {
//~^ ERROR `let` expressions are not supported here
}

if (let Some(a) = opt) && true {
//~^ ERROR `let` expressions are not supported here
}
if (let Some(a) = opt) && (let Some(b) = a) {
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
}
if let Some(a) = opt && (true && true) {
}

if (let Some(a) = opt && (let Some(b) = a)) && b == 1 {
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
}
if (let Some(a) = opt && (let Some(b) = a)) && true {
//~^ ERROR `let` expressions are not supported here
//~| ERROR `let` expressions are not supported here
}
if (let Some(a) = opt && (true)) && true {
//~^ ERROR `let` expressions are not supported here
}

if (true && (true)) && let Some(a) = opt {
}
if (true) && let Some(a) = opt {
}
if true && let Some(a) = opt {
}

let fun = || true;
if let true = (true && fun()) && (true) {
}
}
Loading

0 comments on commit 2ad701e

Please sign in to comment.