From f186e53db9ac0e3fe591a1b929b4ea2aaa7d3598 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Wed, 19 Nov 2025 20:59:22 +0100 Subject: [PATCH] rustc_borrowck: Don't suggest changing closure param type not under user control All removed suggestions in tests were invalid. --- .../src/diagnostics/mutability_errors.rs | 60 +++++++++++++++++++ .../issue-115259-suggest-iter-mut.stderr | 4 +- .../issue-62387-suggest-iter-mut-2.stderr | 4 +- .../issue-62387-suggest-iter-mut.stderr | 8 +-- .../borrowck/option-inspect-mutation.stderr | 4 -- .../suggest-as-ref-on-mut-closure.stderr | 4 +- 6 files changed, 65 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index c06e3c8b3c17..7d72de7efa4a 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1198,6 +1198,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ); return; } + + // Do not suggest changing type if that is not under user control. + if self.is_closure_arg_with_non_locally_decided_type(local) { + return; + } + let decl_span = local_decl.source_info.span; let (amp_mut_sugg, local_var_ty_info) = match *local_decl.local_info() { @@ -1500,6 +1506,60 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { Applicability::HasPlaceholders, ); } + + /// Returns `true` if `local` is an argument in a closure passed to a + /// function defined in another crate. + /// + /// For example, in the following code this function returns `true` for `x` + /// since `Option::inspect()` is not defined in the current crate: + /// + /// ```text + /// some_option.as_mut().inspect(|x| { + /// ``` + fn is_closure_arg_with_non_locally_decided_type(&self, local: Local) -> bool { + // We don't care about regular local variables, only args. + if self.body.local_kind(local) != LocalKind::Arg { + return false; + } + + // Make sure we are inside a closure. + let InstanceKind::Item(body_def_id) = self.body.source.instance else { + return false; + }; + let Some(Node::Expr(hir::Expr { hir_id: body_hir_id, kind, .. })) = + self.infcx.tcx.hir_get_if_local(body_def_id) + else { + return false; + }; + let ExprKind::Closure(hir::Closure { kind: hir::ClosureKind::Closure, .. }) = kind else { + return false; + }; + + // Check if the method/function that our closure is passed to is defined + // in another crate. + let Node::Expr(closure_parent) = self.infcx.tcx.parent_hir_node(*body_hir_id) else { + return false; + }; + match closure_parent.kind { + ExprKind::MethodCall(method, _, _, _) => self + .infcx + .tcx + .typeck(method.hir_id.owner.def_id) + .type_dependent_def_id(closure_parent.hir_id) + .is_some_and(|def_id| !def_id.is_local()), + ExprKind::Call(func, _) => self + .infcx + .tcx + .typeck(func.hir_id.owner.def_id) + .node_type_opt(func.hir_id) + .and_then(|ty| match ty.kind() { + ty::FnDef(def_id, _) => Some(def_id), + _ => None, + }) + .is_some_and(|def_id| !def_id.is_local()), + _ => false, + } + } } struct BindingFinder { diff --git a/tests/ui/borrowck/issue-115259-suggest-iter-mut.stderr b/tests/ui/borrowck/issue-115259-suggest-iter-mut.stderr index 8191f1f4394b..60ff010193fd 100644 --- a/tests/ui/borrowck/issue-115259-suggest-iter-mut.stderr +++ b/tests/ui/borrowck/issue-115259-suggest-iter-mut.stderr @@ -2,9 +2,7 @@ error[E0596]: cannot borrow `**layer` as mutable, as it is behind a `&` referenc --> $DIR/issue-115259-suggest-iter-mut.rs:15:65 | LL | self.layers.iter().fold(0, |result, mut layer| result + layer.process()) - | --------- ^^^^^ `layer` is a `&` reference, so it cannot be borrowed as mutable - | | - | consider changing this binding's type to be: `&mut Box` + | ^^^^^ `layer` is a `&` reference, so it cannot be borrowed as mutable | help: you may want to use `iter_mut` here | diff --git a/tests/ui/borrowck/issue-62387-suggest-iter-mut-2.stderr b/tests/ui/borrowck/issue-62387-suggest-iter-mut-2.stderr index 29121b85f3a0..ba765f06a2c8 100644 --- a/tests/ui/borrowck/issue-62387-suggest-iter-mut-2.stderr +++ b/tests/ui/borrowck/issue-62387-suggest-iter-mut-2.stderr @@ -2,9 +2,7 @@ error[E0596]: cannot borrow `*container` as mutable, as it is behind a `&` refer --> $DIR/issue-62387-suggest-iter-mut-2.rs:30:45 | LL | vec.iter().flat_map(|container| container.things()).cloned().collect::>(); - | --------- ^^^^^^^^^ `container` is a `&` reference, so it cannot be borrowed as mutable - | | - | consider changing this binding's type to be: `&mut Container` + | ^^^^^^^^^ `container` is a `&` reference, so it cannot be borrowed as mutable | help: you may want to use `iter_mut` here | diff --git a/tests/ui/borrowck/issue-62387-suggest-iter-mut.stderr b/tests/ui/borrowck/issue-62387-suggest-iter-mut.stderr index 55c17ab6cdea..677d71d85580 100644 --- a/tests/ui/borrowck/issue-62387-suggest-iter-mut.stderr +++ b/tests/ui/borrowck/issue-62387-suggest-iter-mut.stderr @@ -2,9 +2,7 @@ error[E0596]: cannot borrow `*a` as mutable, as it is behind a `&` reference --> $DIR/issue-62387-suggest-iter-mut.rs:18:27 | LL | v.iter().for_each(|a| a.double()); - | - ^ `a` is a `&` reference, so it cannot be borrowed as mutable - | | - | consider changing this binding's type to be: `&mut A` + | ^ `a` is a `&` reference, so it cannot be borrowed as mutable | help: you may want to use `iter_mut` here | @@ -15,9 +13,7 @@ error[E0596]: cannot borrow `*a` as mutable, as it is behind a `&` reference --> $DIR/issue-62387-suggest-iter-mut.rs:25:39 | LL | v.iter().rev().rev().for_each(|a| a.double()); - | - ^ `a` is a `&` reference, so it cannot be borrowed as mutable - | | - | consider changing this binding's type to be: `&mut A` + | ^ `a` is a `&` reference, so it cannot be borrowed as mutable | help: you may want to use `iter_mut` here | diff --git a/tests/ui/borrowck/option-inspect-mutation.stderr b/tests/ui/borrowck/option-inspect-mutation.stderr index 8c123a5d5e8b..1b6167b00e7e 100644 --- a/tests/ui/borrowck/option-inspect-mutation.stderr +++ b/tests/ui/borrowck/option-inspect-mutation.stderr @@ -1,16 +1,12 @@ error[E0594]: cannot assign to `some_struct.field`, which is behind a `&` reference --> $DIR/option-inspect-mutation.rs:10:9 | -LL | some_struct.as_mut().inspect(|some_struct| { - | ----------- consider changing this binding's type to be: `&mut &mut Struct` LL | some_struct.field *= 10; | ^^^^^^^^^^^^^^^^^^^^^^^ `some_struct` is a `&` reference, so it cannot be written to error[E0594]: cannot assign to `some_struct.field`, which is behind a `&` reference --> $DIR/option-inspect-mutation.rs:16:9 | -LL | Option::inspect(some_struct.as_mut(), |some_struct| { - | ----------- consider changing this binding's type to be: `&mut &mut Struct` LL | some_struct.field *= 20; | ^^^^^^^^^^^^^^^^^^^^^^^ `some_struct` is a `&` reference, so it cannot be written to diff --git a/tests/ui/borrowck/suggest-as-ref-on-mut-closure.stderr b/tests/ui/borrowck/suggest-as-ref-on-mut-closure.stderr index ca0d0bd8a3ca..9b4061af967e 100644 --- a/tests/ui/borrowck/suggest-as-ref-on-mut-closure.stderr +++ b/tests/ui/borrowck/suggest-as-ref-on-mut-closure.stderr @@ -18,9 +18,7 @@ error[E0596]: cannot borrow `*cb` as mutable, as it is behind a `&` reference --> $DIR/suggest-as-ref-on-mut-closure.rs:12:26 | LL | cb.as_ref().map(|cb| cb()); - | -- ^^ `cb` is a `&` reference, so it cannot be borrowed as mutable - | | - | consider changing this binding's type to be: `&mut &mut dyn FnMut()` + | ^^ `cb` is a `&` reference, so it cannot be borrowed as mutable error: aborting due to 2 previous errors