bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #113599 - chenyukang:yukang-fix-use-maybe_body_owned_by, r=cjgillot

Browse source 
Use maybe_body_owned_by for multiple suggestions

This is a continued work from https://github.com/rust-lang/rust/pull/113567

We have several other suggestions not working for closure, this PR use `maybe_body_owned_by` to fix them and add test cases for them.
This commit is contained in:
Matthias Krüger 2023-07-14 19:33:26 +02:00 • committed by GitHub
commit f6dbf7d69b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 280 additions and 118 deletions
Download patch file Download diff file Expand all files Collapse all files

37
compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file

@ -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) {
@ -461,7 +452,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} = move_spans {
// We already suggest cloning for these cases in `explain_captures`.
} else {
self.suggest_cloning(err, ty, move_span);
self.suggest_cloning(err, ty, expr, move_span);
}
}
}
@ -736,9 +727,21 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
true
}
fn suggest_cloning(&self, err: &mut Diagnostic, ty: Ty<'tcx>, span: Span) {
fn suggest_cloning(
&self,
err: &mut Diagnostic,
ty: Ty<'tcx>,
expr: &hir::Expr<'_>,
span: Span,
) {
let tcx = self.infcx.tcx;
// Try to find predicates on *generic params* that would allow copying `ty`
let suggestion =
if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
format!(": {}.clone()", symbol)
} else {
".clone()".to_owned()
};
if let Some(clone_trait_def) = tcx.lang_items().clone_trait()
&& self.infcx
.type_implements_trait(
@ -751,7 +754,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
err.span_suggestion_verbose(
span.shrink_to_hi(),
"consider cloning the value if the performance cost is acceptable",
".clone()",
suggestion,
Applicability::MachineApplicable,
);
}

35
compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View file

@ -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::{
@ -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 };
@ -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:
@ -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
@ -1102,10 +1084,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.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 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 {
@ -1117,6 +1097,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
} else {
None
};
if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
{

41
compiler/rustc_hir_typeck/src/demand.rs
View file

@ -15,7 +15,7 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, Article, AssocItem, Ty, TypeAndMut, TypeFoldable};
use rustc_span::symbol::{sym, Symbol};
use rustc_span::symbol::sym;
use rustc_span::{BytePos, Span, DUMMY_SP};
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::ObligationCause;
@ -997,7 +997,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.collect();
let suggestions_for = |variant: &_, ctor_kind, field_name| {
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
let prefix = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{ident}: "),
None => String::new(),
};
@ -1240,39 +1240,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
pub(crate) fn maybe_get_struct_pattern_shorthand_field(
&self,
expr: &hir::Expr<'_>,
) -> Option<Symbol> {
let hir = self.tcx.hir();
let local = match expr {
hir::Expr {
kind:
hir::ExprKind::Path(hir::QPath::Resolved(
None,
hir::Path {
res: hir::def::Res::Local(_),
segments: [hir::PathSegment { ident, .. }],
..
},
)),
..
} => Some(ident),
_ => None,
}?;
match hir.find_parent(expr.hir_id)? {
Node::ExprField(field) => {
if field.ident.name == local.name && field.is_shorthand {
return Some(local.name);
}
}
_ => {}
}
None
}
/// If the given `HirId` corresponds to a block with a trailing expression, return that expression
pub(crate) fn maybe_get_block_expr(
&self,
@ -1467,7 +1434,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
));
}
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
let prefix = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{ident}: "),
None => String::new(),
};
@ -1661,7 +1628,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
};
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
let prefix = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{ident}: "),
None => String::new(),
};

6
compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file

@ -395,7 +395,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
vec![(expr.span.shrink_to_hi(), format!(".{}()", conversion_method.name))]
};
let struct_pat_shorthand_field =
self.maybe_get_struct_pattern_shorthand_field(expr);
self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr);
if let Some(name) = struct_pat_shorthand_field {
sugg.insert(0, (expr.span.shrink_to_lo(), format!("{}: ", name)));
}
@ -1069,7 +1069,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
.must_apply_modulo_regions()
{
let suggestion = match self.maybe_get_struct_pattern_shorthand_field(expr) {
let suggestion = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!(": {}.clone()", ident),
None => ".clone()".to_string()
};
@ -1247,7 +1247,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return false;
}
let suggestion = match self.maybe_get_struct_pattern_shorthand_field(expr) {
let suggestion = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!(": {}.is_some()", ident),
None => ".is_some()".to_string(),
};

27
compiler/rustc_middle/src/hir/map/mod.rs
View file

@ -1103,6 +1103,33 @@ impl<'hir> Map<'hir> {
_ => None,
}
}
pub fn maybe_get_struct_pattern_shorthand_field(&self, expr: &Expr<'_>) -> Option<Symbol> {
let local = match expr {
Expr {
kind:
ExprKind::Path(QPath::Resolved(
None,
Path {
res: def::Res::Local(_), segments: [PathSegment { ident, .. }], ..
},
)),
..
} => Some(ident),
_ => None,
}?;
match self.find_parent(expr.hir_id)? {
Node::ExprField(field) => {
if field.ident.name == local.name && field.is_shorthand {
return Some(local.name);
}
}
_ => {}
}
None
}
}
impl<'hir> intravisit::Map<'hir> for Map<'hir> {

1
tests/ui/borrowck/copy-suggestion-region-vid.rs
View file

@ -1,3 +1,4 @@
//@run-rustfix
pub struct DataStruct();
pub struct HelperStruct<'n> {

7
tests/ui/borrowck/copy-suggestion-region-vid.stderr
View file

@ -1,5 +1,5 @@
error[E0382]: borrow of moved value: `helpers`
--> $DIR/copy-suggestion-region-vid.rs:12:43
--> $DIR/copy-suggestion-region-vid.rs:13:43
|
LL | let helpers = [vec![], vec![]];
| ------- move occurs because `helpers` has type `[Vec<&i64>; 2]`, which does not implement the `Copy` trait
@ -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: helpers.clone(), is_empty: helpers[0].is_empty() }
| +++++++++++++++++
error: aborting due to previous error

31
tests/ui/borrowck/issue-85765-closure.rs Normal file
View file

@ -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
tests/ui/borrowck/issue-85765-closure.stderr Normal file
View file

@ -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
tests/ui/btreemap/btreemap-index-mut-2.rs Normal file
View file

@ -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
tests/ui/btreemap/btreemap-index-mut-2.stderr Normal file
View file

@ -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
tests/ui/liveness/liveness-move-call-arg-2.rs Normal file
View file

@ -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
tests/ui/liveness/liveness-move-call-arg-2.stderr Normal file
View file

@ -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
tests/ui/suggestions/suggest-mut-method-for-loop-closure.rs Normal file
View file

@ -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
tests/ui/suggestions/suggest-mut-method-for-loop-closure.stderr Normal file
View file

@ -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`.