Skip to content

Commit d4c6128

Browse files
authored
Unrolled build for #149107
Rollup merge of #149107 - Enselic:option-inspect-mutation, r=jieyouxu rustc_borrowck: Don't suggest changing closure param type not under user control This changes output of a handful of tests more than the one added in the first commit, but as far as I can tell, all removed suggestions were invalid. Closes #128381 which is **D-invalid-suggestion** with two 👍-votes.
2 parents 10776a4 + f186e53 commit d4c6128

File tree

7 files changed

+98
-15
lines changed

7 files changed

+98
-15
lines changed

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
11981198
);
11991199
return;
12001200
}
1201+
1202+
// Do not suggest changing type if that is not under user control.
1203+
if self.is_closure_arg_with_non_locally_decided_type(local) {
1204+
return;
1205+
}
1206+
12011207
let decl_span = local_decl.source_info.span;
12021208

12031209
let (amp_mut_sugg, local_var_ty_info) = match *local_decl.local_info() {
@@ -1500,6 +1506,60 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
15001506
Applicability::HasPlaceholders,
15011507
);
15021508
}
1509+
1510+
/// Returns `true` if `local` is an argument in a closure passed to a
1511+
/// function defined in another crate.
1512+
///
1513+
/// For example, in the following code this function returns `true` for `x`
1514+
/// since `Option::inspect()` is not defined in the current crate:
1515+
///
1516+
/// ```text
1517+
/// some_option.as_mut().inspect(|x| {
1518+
/// ```
1519+
fn is_closure_arg_with_non_locally_decided_type(&self, local: Local) -> bool {
1520+
// We don't care about regular local variables, only args.
1521+
if self.body.local_kind(local) != LocalKind::Arg {
1522+
return false;
1523+
}
1524+
1525+
// Make sure we are inside a closure.
1526+
let InstanceKind::Item(body_def_id) = self.body.source.instance else {
1527+
return false;
1528+
};
1529+
let Some(Node::Expr(hir::Expr { hir_id: body_hir_id, kind, .. })) =
1530+
self.infcx.tcx.hir_get_if_local(body_def_id)
1531+
else {
1532+
return false;
1533+
};
1534+
let ExprKind::Closure(hir::Closure { kind: hir::ClosureKind::Closure, .. }) = kind else {
1535+
return false;
1536+
};
1537+
1538+
// Check if the method/function that our closure is passed to is defined
1539+
// in another crate.
1540+
let Node::Expr(closure_parent) = self.infcx.tcx.parent_hir_node(*body_hir_id) else {
1541+
return false;
1542+
};
1543+
match closure_parent.kind {
1544+
ExprKind::MethodCall(method, _, _, _) => self
1545+
.infcx
1546+
.tcx
1547+
.typeck(method.hir_id.owner.def_id)
1548+
.type_dependent_def_id(closure_parent.hir_id)
1549+
.is_some_and(|def_id| !def_id.is_local()),
1550+
ExprKind::Call(func, _) => self
1551+
.infcx
1552+
.tcx
1553+
.typeck(func.hir_id.owner.def_id)
1554+
.node_type_opt(func.hir_id)
1555+
.and_then(|ty| match ty.kind() {
1556+
ty::FnDef(def_id, _) => Some(def_id),
1557+
_ => None,
1558+
})
1559+
.is_some_and(|def_id| !def_id.is_local()),
1560+
_ => false,
1561+
}
1562+
}
15031563
}
15041564

15051565
struct BindingFinder {

tests/ui/borrowck/issue-115259-suggest-iter-mut.stderr

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ error[E0596]: cannot borrow `**layer` as mutable, as it is behind a `&` referenc
22
--> $DIR/issue-115259-suggest-iter-mut.rs:15:65
33
|
44
LL | self.layers.iter().fold(0, |result, mut layer| result + layer.process())
5-
| --------- ^^^^^ `layer` is a `&` reference, so it cannot be borrowed as mutable
6-
| |
7-
| consider changing this binding's type to be: `&mut Box<dyn Layer>`
5+
| ^^^^^ `layer` is a `&` reference, so it cannot be borrowed as mutable
86
|
97
help: you may want to use `iter_mut` here
108
|

tests/ui/borrowck/issue-62387-suggest-iter-mut-2.stderr

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ error[E0596]: cannot borrow `*container` as mutable, as it is behind a `&` refer
22
--> $DIR/issue-62387-suggest-iter-mut-2.rs:30:45
33
|
44
LL | vec.iter().flat_map(|container| container.things()).cloned().collect::<Vec<PathBuf>>();
5-
| --------- ^^^^^^^^^ `container` is a `&` reference, so it cannot be borrowed as mutable
6-
| |
7-
| consider changing this binding's type to be: `&mut Container`
5+
| ^^^^^^^^^ `container` is a `&` reference, so it cannot be borrowed as mutable
86
|
97
help: you may want to use `iter_mut` here
108
|

tests/ui/borrowck/issue-62387-suggest-iter-mut.stderr

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ error[E0596]: cannot borrow `*a` as mutable, as it is behind a `&` reference
22
--> $DIR/issue-62387-suggest-iter-mut.rs:18:27
33
|
44
LL | v.iter().for_each(|a| a.double());
5-
| - ^ `a` is a `&` reference, so it cannot be borrowed as mutable
6-
| |
7-
| consider changing this binding's type to be: `&mut A`
5+
| ^ `a` is a `&` reference, so it cannot be borrowed as mutable
86
|
97
help: you may want to use `iter_mut` here
108
|
@@ -15,9 +13,7 @@ error[E0596]: cannot borrow `*a` as mutable, as it is behind a `&` reference
1513
--> $DIR/issue-62387-suggest-iter-mut.rs:25:39
1614
|
1715
LL | v.iter().rev().rev().for_each(|a| a.double());
18-
| - ^ `a` is a `&` reference, so it cannot be borrowed as mutable
19-
| |
20-
| consider changing this binding's type to be: `&mut A`
16+
| ^ `a` is a `&` reference, so it cannot be borrowed as mutable
2117
|
2218
help: you may want to use `iter_mut` here
2319
|
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//! Regression test for <https://github.com/rust-lang/rust/issues/128381>.
2+
3+
struct Struct {
4+
field: u32,
5+
}
6+
7+
fn main() {
8+
let mut some_struct = Some(Struct { field: 42 });
9+
some_struct.as_mut().inspect(|some_struct| {
10+
some_struct.field *= 10; //~ ERROR cannot assign to `some_struct.field`, which is behind a `&` reference
11+
// Users can't change type of `some_struct` param, so above error must not suggest it.
12+
});
13+
14+
// Same check as above but using `hir::ExprKind::Call` instead of `hir::ExprKind::MethodCall`.
15+
Option::inspect(some_struct.as_mut(), |some_struct| {
16+
some_struct.field *= 20; //~ ERROR cannot assign to `some_struct.field`, which is behind a `&` reference
17+
});
18+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error[E0594]: cannot assign to `some_struct.field`, which is behind a `&` reference
2+
--> $DIR/option-inspect-mutation.rs:10:9
3+
|
4+
LL | some_struct.field *= 10;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^ `some_struct` is a `&` reference, so it cannot be written to
6+
7+
error[E0594]: cannot assign to `some_struct.field`, which is behind a `&` reference
8+
--> $DIR/option-inspect-mutation.rs:16:9
9+
|
10+
LL | some_struct.field *= 20;
11+
| ^^^^^^^^^^^^^^^^^^^^^^^ `some_struct` is a `&` reference, so it cannot be written to
12+
13+
error: aborting due to 2 previous errors
14+
15+
For more information about this error, try `rustc --explain E0594`.

tests/ui/borrowck/suggest-as-ref-on-mut-closure.stderr

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ error[E0596]: cannot borrow `*cb` as mutable, as it is behind a `&` reference
1818
--> $DIR/suggest-as-ref-on-mut-closure.rs:12:26
1919
|
2020
LL | cb.as_ref().map(|cb| cb());
21-
| -- ^^ `cb` is a `&` reference, so it cannot be borrowed as mutable
22-
| |
23-
| consider changing this binding's type to be: `&mut &mut dyn FnMut()`
21+
| ^^ `cb` is a `&` reference, so it cannot be borrowed as mutable
2422

2523
error: aborting due to 2 previous errors
2624

0 commit comments

Comments
 (0)