Skip to content

Commit 9027e40

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#113599 - chenyukang:yukang-fix-use-maybe_body_owned_by, r=cjgillot
Use maybe_body_owned_by for multiple suggestions This is a continued work from rust-lang#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.
2 parents bacf5bc + bdd04a6 commit 9027e40

15 files changed

+280
-118
lines changed

‎compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -363,21 +363,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
363363
}
364364
}
365365
let hir = self.infcx.tcx.hir();
366-
if let Some(hir::Node::Item(hir::Item {
367-
kind: hir::ItemKind::Fn(_, _, body_id),
368-
..
369-
})) = hir.find(self.mir_hir_id())
370-
&& let Some(hir::Node::Expr(expr)) = hir.find(body_id.hir_id)
371-
{
366+
if let Some(body_id) = hir.maybe_body_owned_by(self.mir_def_id()) {
367+
let expr = hir.body(body_id).value;
372368
let place = &self.move_data.move_paths[mpi].place;
373-
let span = place.as_local()
374-
.map(|local| self.body.local_decls[local].source_info.span);
375-
let mut finder = ExpressionFinder {
376-
expr_span: move_span,
377-
expr: None,
378-
pat: None,
379-
parent_pat: None,
380-
};
369+
let span = place.as_local().map(|local| self.body.local_decls[local].source_info.span);
370+
let mut finder =
371+
ExpressionFinder { expr_span: move_span, expr: None, pat: None, parent_pat: None };
381372
finder.visit_expr(expr);
382373
if let Some(span) = span && let Some(expr) = finder.expr {
383374
for (_, expr) in hir.parent_iter(expr.hir_id) {
@@ -461,7 +452,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
461452
} = move_spans {
462453
// We already suggest cloning for these cases in `explain_captures`.
463454
} else {
464-
self.suggest_cloning(err, ty, move_span);
455+
self.suggest_cloning(err, ty, expr, move_span);
465456
}
466457
}
467458
}
@@ -736,9 +727,21 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
736727
true
737728
}
738729

739-
fn suggest_cloning(&self, err: &mut Diagnostic, ty: Ty<'tcx>, span: Span) {
730+
fn suggest_cloning(
731+
&self,
732+
err: &mut Diagnostic,
733+
ty: Ty<'tcx>,
734+
expr: &hir::Expr<'_>,
735+
span: Span,
736+
) {
740737
let tcx = self.infcx.tcx;
741738
// Try to find predicates on *generic params* that would allow copying `ty`
739+
let suggestion =
740+
if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
741+
format!(": {}.clone()", symbol)
742+
} else {
743+
".clone()".to_owned()
744+
};
742745
if let Some(clone_trait_def) = tcx.lang_items().clone_trait()
743746
&& self.infcx
744747
.type_implements_trait(
@@ -751,7 +754,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
751754
err.span_suggestion_verbose(
752755
span.shrink_to_hi(),
753756
"consider cloning the value if the performance cost is acceptable",
754-
".clone()",
757+
suggestion,
755758
Applicability::MachineApplicable,
756759
);
757760
}

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

Lines changed: 41 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
22
use rustc_hir as hir;
33
use rustc_hir::intravisit::Visitor;
44
use rustc_hir::Node;
5-
use rustc_middle::hir::map::Map;
65
use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
76
use rustc_middle::ty::{self, Ty, TyCtxt};
87
use rustc_middle::{
@@ -646,14 +645,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
646645
}
647646
let hir_map = self.infcx.tcx.hir();
648647
let def_id = self.body.source.def_id();
649-
let hir_id = hir_map.local_def_id_to_hir_id(def_id.as_local().unwrap());
650-
let node = hir_map.find(hir_id);
651-
let Some(hir::Node::Item(item)) = node else {
652-
return;
653-
};
654-
let hir::ItemKind::Fn(.., body_id) = item.kind else {
655-
return;
656-
};
648+
let Some(local_def_id) = def_id.as_local() else { return };
649+
let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id) else { return };
657650
let body = self.infcx.tcx.hir().body(body_id);
658651

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

798-
fn maybe_body_id_of_fn(hir_map: Map<'_>, id: HirId) -> Option<BodyId> {
799-
match hir_map.find(id) {
800-
Some(Node::Item(Item { kind: ItemKind::Fn(_, _, body_id), .. }))
801-
| Some(Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(_, body_id), .. })) => {
802-
Some(*body_id)
803-
}
804-
_ => None,
805-
}
806-
}
807790
let hir_map = self.infcx.tcx.hir();
808-
let mir_body_hir_id = self.mir_hir_id();
809-
if let Some(fn_body_id) = maybe_body_id_of_fn(hir_map, mir_body_hir_id) {
791+
if let Some(body_id) = hir_map.maybe_body_owned_by(self.mir_def_id()) {
810792
if let Block(
811793
hir::Block {
812794
expr:
@@ -840,7 +822,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
840822
..
841823
},
842824
_,
843-
) = hir_map.body(fn_body_id).value.kind
825+
) = hir_map.body(body_id).value.kind
844826
{
845827
let opt_suggestions = self
846828
.infcx
@@ -1102,46 +1084,45 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
11021084
}
11031085
let hir_map = self.infcx.tcx.hir();
11041086
let def_id = self.body.source.def_id();
1105-
let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local());
1106-
let node = hir_map.find(hir_id);
1107-
let hir_id = if let Some(hir::Node::Item(item)) = node
1108-
&& let hir::ItemKind::Fn(.., body_id) = item.kind
1109-
{
1110-
let body = hir_map.body(body_id);
1111-
let mut v = BindingFinder {
1112-
span: err_label_span,
1113-
hir_id: None,
1087+
let hir_id = if let Some(local_def_id) = def_id.as_local() &&
1088+
let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id)
1089+
{
1090+
let body = hir_map.body(body_id);
1091+
let mut v = BindingFinder {
1092+
span: err_label_span,
1093+
hir_id: None,
1094+
};
1095+
v.visit_body(body);
1096+
v.hir_id
1097+
} else {
1098+
None
11141099
};
1115-
v.visit_body(body);
1116-
v.hir_id
1117-
} else {
1118-
None
1119-
};
1100+
11201101
if let Some(hir_id) = hir_id
11211102
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
1122-
{
1123-
let (changing, span, sugg) = match local.ty {
1124-
Some(ty) => ("changing", ty.span, message),
1125-
None => (
1126-
"specifying",
1127-
local.pat.span.shrink_to_hi(),
1128-
format!(": {message}"),
1129-
),
1130-
};
1131-
err.span_suggestion_verbose(
1132-
span,
1133-
format!("consider {changing} this binding's type"),
1134-
sugg,
1135-
Applicability::HasPlaceholders,
1136-
);
1137-
} else {
1138-
err.span_label(
1139-
err_label_span,
1140-
format!(
1141-
"consider changing this binding's type to be: `{message}`"
1142-
),
1143-
);
1144-
}
1103+
{
1104+
let (changing, span, sugg) = match local.ty {
1105+
Some(ty) => ("changing", ty.span, message),
1106+
None => (
1107+
"specifying",
1108+
local.pat.span.shrink_to_hi(),
1109+
format!(": {message}"),
1110+
),
1111+
};
1112+
err.span_suggestion_verbose(
1113+
span,
1114+
format!("consider {changing} this binding's type"),
1115+
sugg,
1116+
Applicability::HasPlaceholders,
1117+
);
1118+
} else {
1119+
err.span_label(
1120+
err_label_span,
1121+
format!(
1122+
"consider changing this binding's type to be: `{message}`"
1123+
),
1124+
);
1125+
}
11451126
}
11461127
None => {}
11471128
}

‎compiler/rustc_hir_typeck/src/demand.rs

Lines changed: 4 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError};
1515
use rustc_middle::ty::fold::BottomUpFolder;
1616
use rustc_middle::ty::print::with_no_trimmed_paths;
1717
use rustc_middle::ty::{self, Article, AssocItem, Ty, TypeAndMut, TypeFoldable};
18-
use rustc_span::symbol::{sym, Symbol};
18+
use rustc_span::symbol::sym;
1919
use rustc_span::{BytePos, Span, DUMMY_SP};
2020
use rustc_trait_selection::infer::InferCtxtExt as _;
2121
use rustc_trait_selection::traits::ObligationCause;
@@ -997,7 +997,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
997997
.collect();
998998

999999
let suggestions_for = |variant: &_, ctor_kind, field_name| {
1000-
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
1000+
let prefix = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
10011001
Some(ident) => format!("{ident}: "),
10021002
None => String::new(),
10031003
};
@@ -1240,39 +1240,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12401240
}
12411241
}
12421242

1243-
pub(crate) fn maybe_get_struct_pattern_shorthand_field(
1244-
&self,
1245-
expr: &hir::Expr<'_>,
1246-
) -> Option<Symbol> {
1247-
let hir = self.tcx.hir();
1248-
let local = match expr {
1249-
hir::Expr {
1250-
kind:
1251-
hir::ExprKind::Path(hir::QPath::Resolved(
1252-
None,
1253-
hir::Path {
1254-
res: hir::def::Res::Local(_),
1255-
segments: [hir::PathSegment { ident, .. }],
1256-
..
1257-
},
1258-
)),
1259-
..
1260-
} => Some(ident),
1261-
_ => None,
1262-
}?;
1263-
1264-
match hir.find_parent(expr.hir_id)? {
1265-
Node::ExprField(field) => {
1266-
if field.ident.name == local.name && field.is_shorthand {
1267-
return Some(local.name);
1268-
}
1269-
}
1270-
_ => {}
1271-
}
1272-
1273-
None
1274-
}
1275-
12761243
/// If the given `HirId` corresponds to a block with a trailing expression, return that expression
12771244
pub(crate) fn maybe_get_block_expr(
12781245
&self,
@@ -1467,7 +1434,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14671434
));
14681435
}
14691436

1470-
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
1437+
let prefix = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
14711438
Some(ident) => format!("{ident}: "),
14721439
None => String::new(),
14731440
};
@@ -1661,7 +1628,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16611628
)
16621629
};
16631630

1664-
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
1631+
let prefix = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
16651632
Some(ident) => format!("{ident}: "),
16661633
None => String::new(),
16671634
};

‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
395395
vec![(expr.span.shrink_to_hi(), format!(".{}()", conversion_method.name))]
396396
};
397397
let struct_pat_shorthand_field =
398-
self.maybe_get_struct_pattern_shorthand_field(expr);
398+
self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr);
399399
if let Some(name) = struct_pat_shorthand_field {
400400
sugg.insert(0, (expr.span.shrink_to_lo(), format!("{}: ", name)));
401401
}
@@ -1069,7 +1069,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10691069
)
10701070
.must_apply_modulo_regions()
10711071
{
1072-
let suggestion = match self.maybe_get_struct_pattern_shorthand_field(expr) {
1072+
let suggestion = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
10731073
Some(ident) => format!(": {}.clone()", ident),
10741074
None => ".clone()".to_string()
10751075
};
@@ -1247,7 +1247,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12471247
return false;
12481248
}
12491249

1250-
let suggestion = match self.maybe_get_struct_pattern_shorthand_field(expr) {
1250+
let suggestion = match self.tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
12511251
Some(ident) => format!(": {}.is_some()", ident),
12521252
None => ".is_some()".to_string(),
12531253
};

‎compiler/rustc_middle/src/hir/map/mod.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,33 @@ impl<'hir> Map<'hir> {
11031103
_ => None,
11041104
}
11051105
}
1106+
1107+
pub fn maybe_get_struct_pattern_shorthand_field(&self, expr: &Expr<'_>) -> Option<Symbol> {
1108+
let local = match expr {
1109+
Expr {
1110+
kind:
1111+
ExprKind::Path(QPath::Resolved(
1112+
None,
1113+
Path {
1114+
res: def::Res::Local(_), segments: [PathSegment { ident, .. }], ..
1115+
},
1116+
)),
1117+
..
1118+
} => Some(ident),
1119+
_ => None,
1120+
}?;
1121+
1122+
match self.find_parent(expr.hir_id)? {
1123+
Node::ExprField(field) => {
1124+
if field.ident.name == local.name && field.is_shorthand {
1125+
return Some(local.name);
1126+
}
1127+
}
1128+
_ => {}
1129+
}
1130+
1131+
None
1132+
}
11061133
}
11071134

11081135
impl<'hir> intravisit::Map<'hir> for Map<'hir> {

‎tests/ui/borrowck/copy-suggestion-region-vid.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//@run-rustfix
12
pub struct DataStruct();
23

34
pub struct HelperStruct<'n> {

‎tests/ui/borrowck/copy-suggestion-region-vid.stderr

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0382]: borrow of moved value: `helpers`
2-
--> $DIR/copy-suggestion-region-vid.rs:12:43
2+
--> $DIR/copy-suggestion-region-vid.rs:13:43
33
|
44
LL | let helpers = [vec![], vec![]];
55
| ------- 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() }
88
| ------- ^^^^^^^^^^ value borrowed here after move
99
| |
1010
| value moved here
11+
|
12+
help: consider cloning the value if the performance cost is acceptable
13+
|
14+
LL | HelperStruct { helpers: helpers.clone(), is_empty: helpers[0].is_empty() }
15+
| +++++++++++++++++
1116

1217
error: aborting due to previous error
1318

0 commit comments

Comments
 (0)