From 780349bdafb7983d59d07891abe4225e28d96541 Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Fri, 26 May 2023 02:08:33 +0330 Subject: [PATCH] fix `need-mut` false positive in closure capture of match scrutinee --- crates/hir-def/src/body.rs | 20 ++++++++------ crates/hir-ty/src/infer/closure.rs | 26 +++++++++++++++---- crates/hir-ty/src/layout/tests/closure.rs | 14 +++++++++- .../src/handlers/mutability_errors.rs | 26 +++++++++++++++++++ 4 files changed, 72 insertions(+), 14 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index a387bdbc19e..36626ed1a9b 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -227,9 +227,8 @@ pub fn walk_bindings_in_pat(&self, pat_id: PatId, mut f: impl FnMut(BindingId)) }); } - pub fn walk_pats(&self, pat_id: PatId, f: &mut impl FnMut(PatId)) { + pub fn walk_pats_shallow(&self, pat_id: PatId, mut f: impl FnMut(PatId)) { let pat = &self[pat_id]; - f(pat_id); match pat { Pat::Range { .. } | Pat::Lit(..) @@ -239,23 +238,28 @@ pub fn walk_pats(&self, pat_id: PatId, f: &mut impl FnMut(PatId)) { | Pat::Missing => {} &Pat::Bind { subpat, .. } => { if let Some(subpat) = subpat { - self.walk_pats(subpat, f); + f(subpat); } } Pat::Or(args) | Pat::Tuple { args, .. } | Pat::TupleStruct { args, .. } => { - args.iter().copied().for_each(|p| self.walk_pats(p, f)); + args.iter().copied().for_each(|p| f(p)); } - Pat::Ref { pat, .. } => self.walk_pats(*pat, f), + Pat::Ref { pat, .. } => f(*pat), Pat::Slice { prefix, slice, suffix } => { let total_iter = prefix.iter().chain(slice.iter()).chain(suffix.iter()); - total_iter.copied().for_each(|p| self.walk_pats(p, f)); + total_iter.copied().for_each(|p| f(p)); } Pat::Record { args, .. } => { - args.iter().for_each(|RecordFieldPat { pat, .. }| self.walk_pats(*pat, f)); + args.iter().for_each(|RecordFieldPat { pat, .. }| f(*pat)); } - Pat::Box { inner } => self.walk_pats(*inner, f), + Pat::Box { inner } => f(*inner), } } + + pub fn walk_pats(&self, pat_id: PatId, f: &mut impl FnMut(PatId)) { + f(pat_id); + self.walk_pats_shallow(pat_id, |p| self.walk_pats(p, f)); + } } impl Default for Body { diff --git a/crates/hir-ty/src/infer/closure.rs b/crates/hir-ty/src/infer/closure.rs index 7878ebcc586..787c5c54a29 100644 --- a/crates/hir-ty/src/infer/closure.rs +++ b/crates/hir-ty/src/infer/closure.rs @@ -643,7 +643,21 @@ fn walk_pat(&mut self, result: &mut Option, pat: PatId) { } None => *result = Some(ck), }; - self.body.walk_pats(pat, &mut |p| match &self.body[p] { + + self.walk_pat_inner( + pat, + &mut update_result, + BorrowKind::Mut { allow_two_phase_borrow: false }, + ); + } + + fn walk_pat_inner( + &mut self, + p: PatId, + update_result: &mut impl FnMut(CaptureKind), + mut for_mut: BorrowKind, + ) { + match &self.body[p] { Pat::Ref { .. } | Pat::Box { .. } | Pat::Missing @@ -678,13 +692,15 @@ fn walk_pat(&mut self, result: &mut Option, pat: PatId) { } } crate::BindingMode::Ref(r) => match r { - Mutability::Mut => update_result(CaptureKind::ByRef(BorrowKind::Mut { - allow_two_phase_borrow: false, - })), + Mutability::Mut => update_result(CaptureKind::ByRef(for_mut)), Mutability::Not => update_result(CaptureKind::ByRef(BorrowKind::Shared)), }, }, - }); + } + if self.result.pat_adjustments.get(&p).map_or(false, |x| !x.is_empty()) { + for_mut = BorrowKind::Unique; + } + self.body.walk_pats_shallow(p, |p| self.walk_pat_inner(p, update_result, for_mut)); } fn expr_ty(&self, expr: ExprId) -> Ty { diff --git a/crates/hir-ty/src/layout/tests/closure.rs b/crates/hir-ty/src/layout/tests/closure.rs index 811d6088807..576e7f3fc61 100644 --- a/crates/hir-ty/src/layout/tests/closure.rs +++ b/crates/hir-ty/src/layout/tests/closure.rs @@ -201,7 +201,7 @@ fn match_pattern() { ] |x: i64| { match y { - X(_a, _b, _c) => x, + X(_a, _, _c) => x, } } } @@ -217,6 +217,18 @@ fn match_pattern() { } } } + size_and_align_expr! { + minicore: copy; + stmts: [ + struct X(i64, i32, (u8, i128)); + let y: X = X(2, 5, (7, 3)); + ] + |x: i64| { + match y { + ref _y => x, + } + } + } } #[test] diff --git a/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/crates/ide-diagnostics/src/handlers/mutability_errors.rs index 0af41001808..45b44c2c5ca 100644 --- a/crates/ide-diagnostics/src/handlers/mutability_errors.rs +++ b/crates/ide-diagnostics/src/handlers/mutability_errors.rs @@ -352,6 +352,32 @@ fn clone(mut i: &!) -> ! { ); } + #[test] + fn match_closure_capture() { + check_diagnostics( + r#" +//- minicore: option +fn main() { + let mut v = &mut Some(2); + //^^^^^ 💡 weak: variable does not need to be mutable + let _ = || match v { + Some(k) => { + *k = 5; + } + None => {} + }; + let v = &mut Some(2); + let _ = || match v { + //^ 💡 error: cannot mutate immutable variable `v` + ref mut k => { + *k = &mut Some(5); + } + }; +} +"#, + ); + } + #[test] fn match_bindings() { check_diagnostics(