Skip to content

Commit

Permalink
use maybe_body_owned_by for closure
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Jul 13, 2023
1 parent 7bd81ee commit 3ddf6f7
Show file tree
Hide file tree
Showing 11 changed files with 229 additions and 74 deletions.
19 changes: 5 additions & 14 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,21 +363,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
let hir = self.infcx.tcx.hir();
if let Some(hir::Node::Item(hir::Item {
kind: hir::ItemKind::Fn(_, _, body_id),
..
})) = hir.find(self.mir_hir_id())
&& let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id)
{
if let Some(body_id) = hir.maybe_body_owned_by(self.mir_def_id()) {
let expr = hir.body(body_id).value;
let place = &self.move_data.move_paths[mpi].place;
let span = place.as_local()
.map(|local| self.body.local_decls[local].source_info.span);
let mut finder = ExpressionFinder {
expr_span: move_span,
expr: None,
pat: None,
parent_pat: None,
};
let span = place.as_local().map(|local| self.body.local_decls[local].source_info.span);
let mut finder =
ExpressionFinder { expr_span: move_span, expr: None, pat: None, parent_pat: None };
finder.visit_expr(expr);
if let Some(span) = span && let Some(expr) = finder.expr {
for (_, expr) in hir.parent_iter(expr.hir_id) {
Expand Down
101 changes: 41 additions & 60 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
use rustc_hir as hir;
use rustc_hir::intravisit::Visitor;
use rustc_hir::Node;
use rustc_middle::hir::map::Map;
use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::{
Expand Down Expand Up @@ -646,14 +645,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
let hir_map = self.infcx.tcx.hir();
let def_id = self.body.source.def_id();
let hir_id = hir_map.local_def_id_to_hir_id(def_id.as_local().unwrap());
let node = hir_map.find(hir_id);
let Some(hir::Node::Item(item)) = node else {
return;
};
let hir::ItemKind::Fn(.., body_id) = item.kind else {
return;
};
let Some(local_def_id) = def_id.as_local() else { return };
let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id) else { return };
let body = self.infcx.tcx.hir().body(body_id);

let mut v = V { assign_span: span, err, ty, suggested: false };
Expand Down Expand Up @@ -790,23 +783,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
// In the future, attempt in all path but initially for RHS of for_loop
fn suggest_similar_mut_method_for_for_loop(&self, err: &mut Diagnostic) {
use hir::{
BodyId, Expr,
Expr,
ExprKind::{Block, Call, DropTemps, Match, MethodCall},
HirId, ImplItem, ImplItemKind, Item, ItemKind,
};

fn maybe_body_id_of_fn(hir_map: Map<'_>, id: HirId) -> Option<BodyId> {
match hir_map.find(id) {
Some(Node::Item(Item { kind: ItemKind::Fn(_, _, body_id), .. }))
| Some(Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(_, body_id), .. })) => {
Some(*body_id)
}
_ => None,
}
}
let hir_map = self.infcx.tcx.hir();
let mir_body_hir_id = self.mir_hir_id();
if let Some(fn_body_id) = maybe_body_id_of_fn(hir_map, mir_body_hir_id) {
if let Some(body_id) = hir_map.maybe_body_owned_by(self.mir_def_id()) {
if let Block(
hir::Block {
expr:
Expand Down Expand Up @@ -840,7 +822,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
..
},
_,
) = hir_map.body(fn_body_id).value.kind
) = hir_map.body(body_id).value.kind
{
let opt_suggestions = self
.infcx
Expand Down Expand Up @@ -1102,46 +1084,45 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
let hir_map = self.infcx.tcx.hir();
let def_id = self.body.source.def_id();
let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local());
let node = hir_map.find(hir_id);
let hir_id = if let Some(hir::Node::Item(item)) = node
&& let hir::ItemKind::Fn(.., body_id) = item.kind
{
let body = hir_map.body(body_id);
let mut v = BindingFinder {
span: err_label_span,
hir_id: None,
let hir_id = if let Some(local_def_id) = def_id.as_local() &&
let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id)
{
let body = hir_map.body(body_id);
let mut v = BindingFinder {
span: err_label_span,
hir_id: None,
};
v.visit_body(body);
v.hir_id
} else {
None
};
v.visit_body(body);
v.hir_id
} else {
None
};

if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
{
let (changing, span, sugg) = match local.ty {
Some(ty) => ("changing", ty.span, message),
None => (
"specifying",
local.pat.span.shrink_to_hi(),
format!(": {message}"),
),
};
err.span_suggestion_verbose(
span,
format!("consider {changing} this binding's type"),
sugg,
Applicability::HasPlaceholders,
);
} else {
err.span_label(
err_label_span,
format!(
"consider changing this binding's type to be: `{message}`"
),
);
}
{
let (changing, span, sugg) = match local.ty {
Some(ty) => ("changing", ty.span, message),
None => (
"specifying",
local.pat.span.shrink_to_hi(),
format!(": {message}"),
),
};
err.span_suggestion_verbose(
span,
format!("consider {changing} this binding's type"),
sugg,
Applicability::HasPlaceholders,
);
} else {
err.span_label(
err_label_span,
format!(
"consider changing this binding's type to be: `{message}`"
),
);
}
}
None => {}
}
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/borrowck/copy-suggestion-region-vid.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ LL | HelperStruct { helpers, is_empty: helpers[0].is_empty() }
| ------- ^^^^^^^^^^ value borrowed here after move
| |
| value moved here
|
help: consider cloning the value if the performance cost is acceptable
|
LL | HelperStruct { helpers.clone(), is_empty: helpers[0].is_empty() }
| ++++++++

error: aborting due to previous error

Expand Down
31 changes: 31 additions & 0 deletions tests/ui/borrowck/issue-85765-closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
fn main() {
let _ = || {
let mut test = Vec::new();
let rofl: &Vec<Vec<i32>> = &mut test;
//~^ HELP consider changing this binding's type
rofl.push(Vec::new());
//~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
//~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable

let mut mutvar = 42;
let r = &mutvar;
//~^ HELP consider changing this to be a mutable reference
*r = 0;
//~^ ERROR cannot assign to `*r`, which is behind a `&` reference
//~| NOTE `r` is a `&` reference, so the data it refers to cannot be written

#[rustfmt::skip]
let x: &usize = &mut{0};
//~^ HELP consider changing this binding's type
*x = 1;
//~^ ERROR cannot assign to `*x`, which is behind a `&` reference
//~| NOTE `x` is a `&` reference, so the data it refers to cannot be written

#[rustfmt::skip]
let y: &usize = &mut(0);
//~^ HELP consider changing this binding's type
*y = 1;
//~^ ERROR cannot assign to `*y`, which is behind a `&` reference
//~| NOTE `y` is a `&` reference, so the data it refers to cannot be written
};
}
48 changes: 48 additions & 0 deletions tests/ui/borrowck/issue-85765-closure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
--> $DIR/issue-85765-closure.rs:6:9
|
LL | rofl.push(Vec::new());
| ^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
|
help: consider changing this binding's type
|
LL | let rofl: &mut Vec<Vec<i32>> = &mut test;
| ~~~~~~~~~~~~~~~~~~

error[E0594]: cannot assign to `*r`, which is behind a `&` reference
--> $DIR/issue-85765-closure.rs:13:9
|
LL | *r = 0;
| ^^^^^^ `r` is a `&` reference, so the data it refers to cannot be written
|
help: consider changing this to be a mutable reference
|
LL | let r = &mut mutvar;
| +++

error[E0594]: cannot assign to `*x`, which is behind a `&` reference
--> $DIR/issue-85765-closure.rs:20:9
|
LL | *x = 1;
| ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
|
help: consider changing this binding's type
|
LL | let x: &mut usize = &mut{0};
| ~~~~~~~~~~

error[E0594]: cannot assign to `*y`, which is behind a `&` reference
--> $DIR/issue-85765-closure.rs:27:9
|
LL | *y = 1;
| ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written
|
help: consider changing this binding's type
|
LL | let y: &mut usize = &mut(0);
| ~~~~~~~~~~

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
8 changes: 8 additions & 0 deletions tests/ui/btreemap/btreemap-index-mut-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use std::collections::BTreeMap;

fn main() {
let _ = || {
let mut map = BTreeMap::<u32, u32>::new();
map[&0] = 1; //~ ERROR cannot assign
};
}
19 changes: 19 additions & 0 deletions tests/ui/btreemap/btreemap-index-mut-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0594]: cannot assign to data in an index of `BTreeMap<u32, u32>`
--> $DIR/btreemap-index-mut-2.rs:6:9
|
LL | map[&0] = 1;
| ^^^^^^^^^^^ cannot assign
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
|
LL | map.insert(&0, 1);
| ~~~~~~~~ ~ +
LL | map.get_mut(&0).map(|val| { *val = 1; });
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
LL | let val = map.entry(&0).or_insert(1);
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +

error: aborting due to previous error

For more information about this error, try `rustc --explain E0594`.
12 changes: 12 additions & 0 deletions tests/ui/liveness/liveness-move-call-arg-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
fn take(_x: Box<isize>) {}


fn main() {
let _ = || {
let x: Box<isize> = Box::new(25);

loop {
take(x); //~ ERROR use of moved value: `x`
}
};
}
26 changes: 26 additions & 0 deletions tests/ui/liveness/liveness-move-call-arg-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0382]: use of moved value: `x`
--> $DIR/liveness-move-call-arg-2.rs:9:18
|
LL | let x: Box<isize> = Box::new(25);
| - move occurs because `x` has type `Box<isize>`, which does not implement the `Copy` trait
LL |
LL | loop {
| ---- inside of this loop
LL | take(x);
| ^ value moved here, in previous iteration of loop
|
note: consider changing this parameter type in function `take` to borrow instead if owning the value isn't necessary
--> $DIR/liveness-move-call-arg-2.rs:1:13
|
LL | fn take(_x: Box<isize>) {}
| ---- ^^^^^^^^^^ this parameter takes ownership of the value
| |
| in this function
help: consider cloning the value if the performance cost is acceptable
|
LL | take(x.clone());
| ++++++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0382`.
19 changes: 19 additions & 0 deletions tests/ui/suggestions/suggest-mut-method-for-loop-closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use std::collections::HashMap;
struct X(usize);
struct Y {
v: u32,
}

fn main() {
let _ = || {
let mut buzz = HashMap::new();
buzz.insert("a", Y { v: 0 });

for mut t in buzz.values() {
//~^ HELP
//~| SUGGESTION values_mut()
t.v += 1;
//~^ ERROR cannot assign
}
};
}
15 changes: 15 additions & 0 deletions tests/ui/suggestions/suggest-mut-method-for-loop-closure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0594]: cannot assign to `t.v`, which is behind a `&` reference
--> $DIR/suggest-mut-method-for-loop-closure.rs:15:13
|
LL | for mut t in buzz.values() {
| -------------
| | |
| | help: use mutable method: `values_mut()`
| this iterator yields `&` references
...
LL | t.v += 1;
| ^^^^^^^^ `t` is a `&` reference, so the data it refers to cannot be written

error: aborting due to previous error

For more information about this error, try `rustc --explain E0594`.

0 comments on commit 3ddf6f7

Please sign in to comment.