1
Fork 0

Rollup merge of #136402 - notriddle:notriddle/let-expr-detector, r=compiler-errors

diagnostics: fix borrowck suggestions for if/while let conditionals

This code detects the case where one of the borrows is inside the let init expr while the other end is not. If that happens, we don't want to suggest adding a semicolon, because it won't work.

Fixes #133941
This commit is contained in:
Matthias Krüger 2025-02-02 18:05:23 +01:00 committed by GitHub
commit e066208255
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 227 additions and 18 deletions

View file

@ -248,7 +248,98 @@ impl<'tcx> BorrowExplanation<'tcx> {
);
err.span_label(body.source_info(drop_loc).span, message);
if let LocalInfo::BlockTailTemp(info) = local_decl.local_info() {
struct FindLetExpr<'hir> {
span: Span,
result: Option<(Span, &'hir hir::Pat<'hir>, &'hir hir::Expr<'hir>)>,
tcx: TyCtxt<'hir>,
}
impl<'hir> rustc_hir::intravisit::Visitor<'hir> for FindLetExpr<'hir> {
type NestedFilter = rustc_middle::hir::nested_filter::OnlyBodies;
fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}
fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
if let hir::ExprKind::If(cond, _conseq, _alt)
| hir::ExprKind::Loop(
hir::Block {
expr:
Some(&hir::Expr {
kind: hir::ExprKind::If(cond, _conseq, _alt),
..
}),
..
},
_,
hir::LoopSource::While,
_,
) = expr.kind
&& let hir::ExprKind::Let(hir::LetExpr {
init: let_expr_init,
span: let_expr_span,
pat: let_expr_pat,
..
}) = cond.kind
&& let_expr_init.span.contains(self.span)
{
self.result =
Some((*let_expr_span, let_expr_pat, let_expr_init))
} else {
hir::intravisit::walk_expr(self, expr);
}
}
}
if let &LocalInfo::IfThenRescopeTemp { if_then } = local_decl.local_info()
&& let hir::Node::Expr(expr) = tcx.hir_node(if_then)
&& let hir::ExprKind::If(cond, conseq, alt) = expr.kind
&& let hir::ExprKind::Let(&hir::LetExpr {
span: _,
pat,
init,
// FIXME(#101728): enable rewrite when type ascription is
// stabilized again.
ty: None,
recovered: _,
}) = cond.kind
&& pat.span.can_be_used_for_suggestions()
&& let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span)
{
suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err);
} else if let Some((old, new)) = multiple_borrow_span
&& let def_id = body.source.def_id()
&& let Some(node) = tcx.hir().get_if_local(def_id)
&& let Some(body_id) = node.body_id()
&& let hir_body = tcx.hir().body(body_id)
&& let mut expr_finder = (FindLetExpr { span: old, result: None, tcx })
&& let Some((let_expr_span, let_expr_pat, let_expr_init)) = {
expr_finder.visit_expr(hir_body.value);
expr_finder.result
}
&& !let_expr_span.contains(new)
{
// #133941: The `old` expression is at the conditional part of an
// if/while let expression. Adding a semicolon won't work.
// Instead, try suggesting the `matches!` macro or a temporary.
if let_expr_pat
.walk_short(|pat| !matches!(pat.kind, hir::PatKind::Binding(..)))
{
if let Ok(pat_snippet) =
tcx.sess.source_map().span_to_snippet(let_expr_pat.span)
&& let Ok(init_snippet) =
tcx.sess.source_map().span_to_snippet(let_expr_init.span)
{
err.span_suggestion_verbose(
let_expr_span,
"consider using the `matches!` macro",
format!("matches!({init_snippet}, {pat_snippet})"),
Applicability::MaybeIncorrect,
);
} else {
err.note("consider using the `matches!` macro");
}
}
} else if let LocalInfo::BlockTailTemp(info) = local_decl.local_info() {
if info.tail_result_is_ignored {
// #85581: If the first mutable borrow's scope contains
// the second borrow, this suggestion isn't helpful.
@ -281,23 +372,6 @@ impl<'tcx> BorrowExplanation<'tcx> {
Applicability::MaybeIncorrect,
);
};
} else if let &LocalInfo::IfThenRescopeTemp { if_then } =
local_decl.local_info()
&& let hir::Node::Expr(expr) = tcx.hir_node(if_then)
&& let hir::ExprKind::If(cond, conseq, alt) = expr.kind
&& let hir::ExprKind::Let(&hir::LetExpr {
span: _,
pat,
init,
// FIXME(#101728): enable rewrite when type ascription is
// stabilized again.
ty: None,
recovered: _,
}) = cond.kind
&& pat.span.can_be_used_for_suggestions()
&& let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span)
{
suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err);
}
}
}