Skip to content

Commit

Permalink
Rollup merge of #113599 - chenyukang:yukang-fix-use-maybe_body_owned_…
Browse files Browse the repository at this point in the history 
…by, r=cjgillot

Use maybe_body_owned_by for multiple suggestions

This is a continued work from #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.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jul 14, 2023
2 parents 5767cad + bdd04a6 commit f6dbf7d
Show file tree
Hide file tree
Showing 15 changed files with 280 additions and 118 deletions.
37 changes: 20 additions & 17 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
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 Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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(
Expand All @@ -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,
);
}
Expand Down
101 changes: 41 additions & 60 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View file
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
41 changes: 4 additions & 37 deletions compiler/rustc_hir_typeck/src/demand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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(),
};
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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()
};
Expand Down Expand Up @@ -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(),
};
Expand Down
27 changes: 27 additions & 0 deletions compiler/rustc_middle/src/hir/map/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/borrowck/copy-suggestion-region-vid.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//@run-rustfix
pub struct DataStruct();

pub struct HelperStruct<'n> {
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/borrowck/copy-suggestion-region-vid.stderr
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down
Loading

0 comments on commit f6dbf7d

Please sign in to comment.