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

Account for if (let pat = expr) {} #82854

Merged
merged 4 commits into from
Mar 9, 2021
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
8 changes: 8 additions & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,14 @@ impl Expr {
}
}

pub fn peel_parens(&self) -> &Expr {
let mut expr = self;
while let ExprKind::Paren(inner) = &expr.kind {
expr = &inner;
}
expr
}

/// Attempts to reparse as `Ty` (for diagnostic purposes).
pub fn to_ty(&self) -> Option<P<Ty>> {
let kind = match &self.kind {
Expand Down
42 changes: 40 additions & 2 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,23 @@ impl<'hir> LoweringContext<'_, 'hir> {
ExprKind::Let(ref pat, ref scrutinee) => {
self.lower_expr_if_let(e.span, pat, scrutinee, then, else_opt.as_deref())
}
ExprKind::Paren(ref paren) => match paren.peel_parens().kind {
ExprKind::Let(ref pat, ref scrutinee) => {
// A user has written `if (let Some(x) = foo) {`, we want to avoid
// confusing them with mentions of nightly features.
// If this logic is changed, you will also likely need to touch
// `unused::UnusedParens::check_expr`.
self.if_let_expr_with_parens(cond, &paren.peel_parens());
self.lower_expr_if_let(
e.span,
pat,
scrutinee,
then,
else_opt.as_deref(),
)
}
_ => self.lower_expr_if(cond, then, else_opt.as_deref()),
},
_ => self.lower_expr_if(cond, then, else_opt.as_deref()),
},
ExprKind::While(ref cond, ref body, opt_label) => self
Expand Down Expand Up @@ -346,6 +363,25 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ExprKind::Call(f, self.lower_exprs(&real_args))
}

fn if_let_expr_with_parens(&mut self, cond: &Expr, paren: &Expr) {
let start = cond.span.until(paren.span);
let end = paren.span.shrink_to_hi().until(cond.span.shrink_to_hi());
self.sess
.struct_span_err(
vec![start, end],
"invalid parentheses around `let` expression in `if let`",
)
.multipart_suggestion(
"`if let` needs to be written without parentheses",
vec![(start, String::new()), (end, String::new())],
rustc_errors::Applicability::MachineApplicable,
)
.emit();
// Ideally, we'd remove the feature gating of a `let` expression since we are already
// complaining about it here, but `feature_gate::check_crate` has already run by now:
// self.sess.parse_sess.gated_spans.ungate_last(sym::let_chains, paren.span);
}

/// Emit an error and lower `ast::ExprKind::Let(pat, scrutinee)` into:
/// ```rust
/// match scrutinee { pats => true, _ => false }
Expand All @@ -356,8 +392,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
if self.sess.opts.unstable_features.is_nightly_build() {
self.sess
.struct_span_err(span, "`let` expressions are not supported here")
.note("only supported directly in conditions of `if`- and `while`-expressions")
.note("as well as when nested within `&&` and parenthesis in those conditions")
.note(
"only supported directly without parentheses in conditions of `if`- and \
`while`-expressions, as well as in `let` chains within parentheses",
)
.emit();
} else {
self.sess
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,16 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session) {
}
};
}
gate_all!(if_let_guard, "`if let` guards are experimental");
gate_all!(let_chains, "`let` expressions in this position are experimental");
gate_all!(
if_let_guard,
"`if let` guards are experimental",
"you can write `if matches!(<expr>, <pattern>)` instead of `if let <pattern> = <expr>`"
);
gate_all!(
let_chains,
"`let` expressions in this position are experimental",
"you can write `matches!(<expr>, <pattern>)` instead of `let <pattern> = <expr>`"
);
gate_all!(
async_closure,
"async closures are unstable",
Expand Down
31 changes: 28 additions & 3 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ trait UnusedDelimLint {
use rustc_ast::ExprKind::*;
let (value, ctx, followed_by_block, left_pos, right_pos) = match e.kind {
// Do not lint `unused_braces` in `if let` expressions.
If(ref cond, ref block, ..)
If(ref cond, ref block, _)
if !matches!(cond.kind, Let(_, _)) || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX =>
{
let left = e.span.lo() + rustc_span::BytePos(2);
Expand Down Expand Up @@ -816,8 +816,33 @@ impl UnusedParens {

impl EarlyLintPass for UnusedParens {
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
if let ExprKind::Let(ref pat, ..) | ExprKind::ForLoop(ref pat, ..) = e.kind {
self.check_unused_parens_pat(cx, pat, false, false);
match e.kind {
ExprKind::Let(ref pat, _) | ExprKind::ForLoop(ref pat, ..) => {
self.check_unused_parens_pat(cx, pat, false, false);
}
// We ignore parens in cases like `if (((let Some(0) = Some(1))))` because we already
// handle a hard error for them during AST lowering in `lower_expr_mut`, but we still
// want to complain about things like `if let 42 = (42)`.
ExprKind::If(ref cond, ref block, ref else_)
if matches!(cond.peel_parens().kind, ExprKind::Let(..)) =>
{
self.check_unused_delims_expr(
cx,
cond.peel_parens(),
UnusedDelimsCtx::LetScrutineeExpr,
true,
None,
None,
);
for stmt in &block.stmts {
<Self as UnusedDelimLint>::check_stmt(self, cx, stmt);
}
if let Some(e) = else_ {
<Self as UnusedDelimLint>::check_expr(self, cx, e);
}
return;
}
_ => {}
}

<Self as UnusedDelimLint>::check_expr(self, cx, e)
Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/pattern/issue-82290.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ error: `let` expressions are not supported here
LL | if true && let x = 1 {
| ^^^^^^^^^
|
= note: only supported directly in conditions of `if`- and `while`-expressions
= note: as well as when nested within `&&` and parenthesis in those conditions
= note: only supported directly without parentheses in conditions of `if`- and `while`-expressions, as well as in `let` chains within parentheses

warning: the feature `let_chains` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/issue-82290.rs:1:12
Expand Down
Loading