From 6004968e4a85b1b1634b26c319b6e0cdaa2ed680 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Wed, 19 Nov 2025 20:47:50 +0100 Subject: [PATCH 1/2] tests: Check stderr when trying to mutate inside Option::inspect() --- tests/ui/borrowck/option-inspect-mutation.rs | 18 ++++++++++++++++++ .../borrowck/option-inspect-mutation.stderr | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/ui/borrowck/option-inspect-mutation.rs create mode 100644 tests/ui/borrowck/option-inspect-mutation.stderr diff --git a/tests/ui/borrowck/option-inspect-mutation.rs b/tests/ui/borrowck/option-inspect-mutation.rs new file mode 100644 index 0000000000000..038d06e7e6a91 --- /dev/null +++ b/tests/ui/borrowck/option-inspect-mutation.rs @@ -0,0 +1,18 @@ +//! Regression test for . + +struct Struct { + field: u32, +} + +fn main() { + let mut some_struct = Some(Struct { field: 42 }); + some_struct.as_mut().inspect(|some_struct| { + some_struct.field *= 10; //~ ERROR cannot assign to `some_struct.field`, which is behind a `&` reference + // Users can't change type of `some_struct` param, so above error must not suggest it. + }); + + // Same check as above but using `hir::ExprKind::Call` instead of `hir::ExprKind::MethodCall`. + Option::inspect(some_struct.as_mut(), |some_struct| { + some_struct.field *= 20; //~ ERROR cannot assign to `some_struct.field`, which is behind a `&` reference + }); +} diff --git a/tests/ui/borrowck/option-inspect-mutation.stderr b/tests/ui/borrowck/option-inspect-mutation.stderr new file mode 100644 index 0000000000000..8c123a5d5e8b1 --- /dev/null +++ b/tests/ui/borrowck/option-inspect-mutation.stderr @@ -0,0 +1,19 @@ +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 + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0594`. From f186e53db9ac0e3fe591a1b929b4ea2aaa7d3598 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Wed, 19 Nov 2025 20:59:22 +0100 Subject: [PATCH 2/2] 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 c06e3c8b3c174..7d72de7efa4a1 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 8191f1f4394b7..60ff010193fd2 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 29121b85f3a08..ba765f06a2c8f 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 55c17ab6cdea1..677d71d85580d 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 8c123a5d5e8b1..1b6167b00e7e7 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 ca0d0bd8a3caa..9b4061af967ed 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