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.
This commit is contained in:
Michael Howell
parent
e08cd3cf05
commit
ecb2d5c43d
3 changed files with 227 additions and 18 deletions
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,46 @@
|
|||
// https://github.com/rust-lang/rust/issues/133941
|
||||
use std::marker::PhantomData;
|
||||
|
||||
struct Bar<'a>(PhantomData<&'a mut i32>);
|
||||
|
||||
impl<'a> Drop for Bar<'a> {
|
||||
fn drop(&mut self) {}
|
||||
}
|
||||
|
||||
struct Foo();
|
||||
|
||||
impl Foo {
|
||||
fn f(&mut self) -> Option<Bar<'_>> {
|
||||
None
|
||||
}
|
||||
|
||||
fn g(&mut self) {}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let mut foo = Foo();
|
||||
while let Some(_) = foo.f() {
|
||||
//~^ HELP matches!
|
||||
foo.g();
|
||||
//~^ ERROR [E0499]
|
||||
}
|
||||
if let Some(_) = foo.f() {
|
||||
//~^ HELP matches!
|
||||
foo.g();
|
||||
//~^ ERROR [E0499]
|
||||
}
|
||||
while let Some(_x) = foo.f() {
|
||||
foo.g();
|
||||
//~^ ERROR [E0499]
|
||||
}
|
||||
if let Some(_x) = foo.f() {
|
||||
foo.g();
|
||||
//~^ ERROR [E0499]
|
||||
}
|
||||
while let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
|
||||
//~^ ERROR [E0499]
|
||||
}
|
||||
if let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
|
||||
//~^ ERROR [E0499]
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,89 @@
|
|||
error[E0499]: cannot borrow `foo` as mutable more than once at a time
|
||||
--> $DIR/already-borrowed-as-mutable-if-let-133941.rs:24:9
|
||||
|
|
||||
LL | while let Some(_) = foo.f() {
|
||||
| -------
|
||||
| |
|
||||
| first mutable borrow occurs here
|
||||
| a temporary with access to the first borrow is created here ...
|
||||
LL |
|
||||
LL | foo.g();
|
||||
| ^^^ second mutable borrow occurs here
|
||||
LL |
|
||||
LL | }
|
||||
| - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
|
||||
|
|
||||
help: consider using the `matches!` macro
|
||||
|
|
||||
LL | while matches!(foo.f(), Some(_)) {
|
||||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
error[E0499]: cannot borrow `foo` as mutable more than once at a time
|
||||
--> $DIR/already-borrowed-as-mutable-if-let-133941.rs:29:9
|
||||
|
|
||||
LL | if let Some(_) = foo.f() {
|
||||
| -------
|
||||
| |
|
||||
| first mutable borrow occurs here
|
||||
| a temporary with access to the first borrow is created here ...
|
||||
LL |
|
||||
LL | foo.g();
|
||||
| ^^^ second mutable borrow occurs here
|
||||
LL |
|
||||
LL | }
|
||||
| - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
|
||||
|
|
||||
help: consider using the `matches!` macro
|
||||
|
|
||||
LL | if matches!(foo.f(), Some(_)) {
|
||||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
error[E0499]: cannot borrow `foo` as mutable more than once at a time
|
||||
--> $DIR/already-borrowed-as-mutable-if-let-133941.rs:33:9
|
||||
|
|
||||
LL | while let Some(_x) = foo.f() {
|
||||
| -------
|
||||
| |
|
||||
| first mutable borrow occurs here
|
||||
| a temporary with access to the first borrow is created here ...
|
||||
LL | foo.g();
|
||||
| ^^^ second mutable borrow occurs here
|
||||
LL |
|
||||
LL | }
|
||||
| - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
|
||||
|
||||
error[E0499]: cannot borrow `foo` as mutable more than once at a time
|
||||
--> $DIR/already-borrowed-as-mutable-if-let-133941.rs:37:9
|
||||
|
|
||||
LL | if let Some(_x) = foo.f() {
|
||||
| -------
|
||||
| |
|
||||
| first mutable borrow occurs here
|
||||
| a temporary with access to the first borrow is created here ...
|
||||
LL | foo.g();
|
||||
| ^^^ second mutable borrow occurs here
|
||||
LL |
|
||||
LL | }
|
||||
| - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
|
||||
|
||||
error[E0499]: cannot borrow `foo` as mutable more than once at a time
|
||||
--> $DIR/already-borrowed-as-mutable-if-let-133941.rs:40:45
|
||||
|
|
||||
LL | while let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
|
||||
| --- ^^^ - first borrow might be used here, when `_x` is dropped and runs the destructor for type `Option<Bar<'_>>`
|
||||
| | |
|
||||
| | second mutable borrow occurs here
|
||||
| first mutable borrow occurs here
|
||||
|
||||
error[E0499]: cannot borrow `foo` as mutable more than once at a time
|
||||
--> $DIR/already-borrowed-as-mutable-if-let-133941.rs:43:42
|
||||
|
|
||||
LL | if let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
|
||||
| --- ^^^ - first borrow might be used here, when `_x` is dropped and runs the destructor for type `Option<Bar<'_>>`
|
||||
| | |
|
||||
| | second mutable borrow occurs here
|
||||
| first mutable borrow occurs here
|
||||
|
||||
error: aborting due to 6 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0499`.
|
||||
Loading…
Add table
Add a link
Reference in a new issue