mirror of
https://github.com/rust-lang/rust.git
synced 2025-01-22 12:43:36 +00:00
Rollup merge of #132172 - dianne:suggest-borrow-generic, r=matthewjasper
borrowck diagnostics: suggest borrowing function inputs in generic positions # Summary This generalizes borrowck's existing suggestions to borrow instead of moving when passing by-value to a function that's generic in that input. Previously, this was special-cased to `AsRef`/`Borrow`-like traits and `Fn`-like traits. This PR changes it to test if, for a moved place with type `T`, that the callee's signature and clauses don't break if you substitute in `&T` or `&mut T`. For instance, it now works with `Read`/`Write`-like traits. Fixes https://github.com/rust-lang/rust/issues/131413 # Incidental changes - No longer spuriously suggests mutable borrows of closures in some situations (see e.g. the tests in [tests/ui/closures/2229_closure_analysis/](https://github.com/rust-lang/rust/compare/master...dianne:rust:suggest-borrow-generic?expand=1#diff-8dfb200c559f0995d0f2ffa2f23bc6f8041b263e264e5c329a1f4171769787c0)). - No longer suggests cloning closures that implement `Fn`, since they can be borrowed (see e.g. [tests/ui/moves/borrow-closures-instead-of-move.stderr](https://github.com/rust-lang/rust/compare/master...dianne:rust:suggest-borrow-generic?expand=1#diff-5db268aac405eec56d099a72d8b58ac46dab523cf013e29008104840168577fb)). This keeps the behavior to suppress suggestions of `fn_once.clone()()`. I think it might make sense to suggest it with a "but this might not be your desired behavior" caveat, as is done when something is used after being consumed as the receiver for a method call. That's probably out of the scope of this PR though. # Limitations and possible improvements - This doesn't work for receivers of method calls. This is a small change, and I have it implemented locally, but I'm not sure it's useful on its own. In most cases I've found borrowing the receiver would change the call's output type (see below). In other cases (e.g. `Iterator::sum`), borrowing the receiver isn't useful since it's consumed. - This doesn't work when it would change the call's output type. In general, I figure inserting references into the output type is an unwanted change. However, this also means it doesn't work in cases where the new output type would be effectively the same as the old one. For example, from the rand crate, the iterator returned by [`Rng::sample_iter`](https://docs.rs/rand/latest/rand/trait.Rng.html#method.sample_iter) is effectively the same (modulo regions) whether you borrow or consume the receiver `Rng`, so common usage involves borrowing it. I'm not sure whether the best approach is to add a more complex check of approximate equivalence, to forego checking the call's output type and give spurious suggestions, or to leave it as-is. - This doesn't work when it would change the call's other input types. Instead, it could suggest borrowing any others that have the same parameter type (but only when suggesting shared borrows). I think this would be a pretty easy change, but I don't think it's very useful so long as the normalized output type can't change. I'm happy to implement any of these (or other potential improvements to this), but I'm not sure which are common enough patterns to justify the added complexity. Please let me know if any sound worthwhile.
This commit is contained in:
commit
5ee347ece4
@ -27,7 +27,7 @@ use rustc_middle::mir::{
|
||||
};
|
||||
use rustc_middle::ty::print::PrintTraitRefExt as _;
|
||||
use rustc_middle::ty::{
|
||||
self, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast,
|
||||
self, ClauseKind, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast,
|
||||
suggest_constraining_type_params,
|
||||
};
|
||||
use rustc_middle::util::CallKind;
|
||||
@ -39,6 +39,7 @@ use rustc_span::{BytePos, Span, Symbol};
|
||||
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
|
||||
use rustc_trait_selection::error_reporting::traits::FindExprBySpan;
|
||||
use rustc_trait_selection::infer::InferCtxtExt;
|
||||
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
|
||||
use rustc_trait_selection::traits::{Obligation, ObligationCause, ObligationCtxt};
|
||||
use tracing::{debug, instrument};
|
||||
|
||||
@ -201,16 +202,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
|
||||
let mut has_suggest_reborrow = false;
|
||||
if !seen_spans.contains(&move_span) {
|
||||
if !closure {
|
||||
self.suggest_ref_or_clone(
|
||||
mpi,
|
||||
&mut err,
|
||||
&mut in_pattern,
|
||||
move_spans,
|
||||
moved_place.as_ref(),
|
||||
&mut has_suggest_reborrow,
|
||||
);
|
||||
}
|
||||
self.suggest_ref_or_clone(
|
||||
mpi,
|
||||
&mut err,
|
||||
&mut in_pattern,
|
||||
move_spans,
|
||||
moved_place.as_ref(),
|
||||
&mut has_suggest_reborrow,
|
||||
closure,
|
||||
);
|
||||
|
||||
let msg_opt = CapturedMessageOpt {
|
||||
is_partial_move,
|
||||
@ -266,27 +266,11 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
}
|
||||
}
|
||||
|
||||
let opt_name = self.describe_place_with_options(place.as_ref(), DescribePlaceOpt {
|
||||
including_downcast: true,
|
||||
including_tuple_field: true,
|
||||
});
|
||||
let note_msg = match opt_name {
|
||||
Some(name) => format!("`{name}`"),
|
||||
None => "value".to_owned(),
|
||||
};
|
||||
if self.suggest_borrow_fn_like(&mut err, ty, &move_site_vec, ¬e_msg)
|
||||
|| if let UseSpans::FnSelfUse { kind, .. } = use_spans
|
||||
&& let CallKind::FnCall { fn_trait_id, self_ty } = kind
|
||||
&& let ty::Param(_) = self_ty.kind()
|
||||
&& ty == self_ty
|
||||
&& self.infcx.tcx.is_lang_item(fn_trait_id, LangItem::FnOnce)
|
||||
{
|
||||
// this is a type parameter `T: FnOnce()`, don't suggest `T: FnOnce() + Clone`.
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
{
|
||||
if self.param_env.caller_bounds().iter().any(|c| {
|
||||
c.as_trait_clause().is_some_and(|pred| {
|
||||
pred.skip_binder().self_ty() == ty && self.infcx.tcx.is_fn_trait(pred.def_id())
|
||||
})
|
||||
}) {
|
||||
// Suppress the next suggestion since we don't want to put more bounds onto
|
||||
// something that already has `Fn`-like bounds (or is a closure), so we can't
|
||||
// restrict anyways.
|
||||
@ -295,6 +279,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
self.suggest_adding_bounds(&mut err, ty, copy_did, span);
|
||||
}
|
||||
|
||||
let opt_name = self.describe_place_with_options(place.as_ref(), DescribePlaceOpt {
|
||||
including_downcast: true,
|
||||
including_tuple_field: true,
|
||||
});
|
||||
let note_msg = match opt_name {
|
||||
Some(name) => format!("`{name}`"),
|
||||
None => "value".to_owned(),
|
||||
};
|
||||
if needs_note {
|
||||
if let Some(local) = place.as_local() {
|
||||
let span = self.body.local_decls[local].source_info.span;
|
||||
@ -341,6 +333,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
move_spans: UseSpans<'tcx>,
|
||||
moved_place: PlaceRef<'tcx>,
|
||||
has_suggest_reborrow: &mut bool,
|
||||
moved_or_invoked_closure: bool,
|
||||
) {
|
||||
let move_span = match move_spans {
|
||||
UseSpans::ClosureUse { capture_kind_span, .. } => capture_kind_span,
|
||||
@ -428,104 +421,76 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
}
|
||||
let typeck = self.infcx.tcx.typeck(self.mir_def_id());
|
||||
let parent = self.infcx.tcx.parent_hir_node(expr.hir_id);
|
||||
let (def_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent
|
||||
let (def_id, call_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent
|
||||
&& let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind
|
||||
{
|
||||
(typeck.type_dependent_def_id(parent_expr.hir_id), args, 1)
|
||||
let def_id = typeck.type_dependent_def_id(parent_expr.hir_id);
|
||||
(def_id, Some(parent_expr.hir_id), args, 1)
|
||||
} else if let hir::Node::Expr(parent_expr) = parent
|
||||
&& let hir::ExprKind::Call(call, args) = parent_expr.kind
|
||||
&& let ty::FnDef(def_id, _) = typeck.node_type(call.hir_id).kind()
|
||||
{
|
||||
(Some(*def_id), args, 0)
|
||||
(Some(*def_id), Some(call.hir_id), args, 0)
|
||||
} else {
|
||||
(None, &[][..], 0)
|
||||
(None, None, &[][..], 0)
|
||||
};
|
||||
let ty = place.ty(self.body, self.infcx.tcx).ty;
|
||||
|
||||
// If the moved value is a mut reference, it is used in a
|
||||
// generic function and it's type is a generic param, it can be
|
||||
// reborrowed to avoid moving.
|
||||
// for example:
|
||||
// struct Y(u32);
|
||||
// x's type is '& mut Y' and it is used in `fn generic<T>(x: T) {}`.
|
||||
let mut can_suggest_clone = true;
|
||||
if let Some(def_id) = def_id
|
||||
&& self.infcx.tcx.def_kind(def_id).is_fn_like()
|
||||
&& let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
|
||||
&& let Some(arg) = self
|
||||
.infcx
|
||||
.tcx
|
||||
.fn_sig(def_id)
|
||||
.skip_binder()
|
||||
.skip_binder()
|
||||
.inputs()
|
||||
.get(pos + offset)
|
||||
&& let ty::Param(_) = arg.kind()
|
||||
{
|
||||
let place = &self.move_data.move_paths[mpi].place;
|
||||
let ty = place.ty(self.body, self.infcx.tcx).ty;
|
||||
if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() {
|
||||
// The move occurred as one of the arguments to a function call. Is that
|
||||
// argument generic? `def_id` can't be a closure here, so using `fn_sig` is fine
|
||||
let arg_param = if self.infcx.tcx.def_kind(def_id).is_fn_like()
|
||||
&& let sig =
|
||||
self.infcx.tcx.fn_sig(def_id).instantiate_identity().skip_binder()
|
||||
&& let Some(arg_ty) = sig.inputs().get(pos + offset)
|
||||
&& let ty::Param(arg_param) = arg_ty.kind()
|
||||
{
|
||||
Some(arg_param)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// If the moved value is a mut reference, it is used in a
|
||||
// generic function and it's type is a generic param, it can be
|
||||
// reborrowed to avoid moving.
|
||||
// for example:
|
||||
// struct Y(u32);
|
||||
// x's type is '& mut Y' and it is used in `fn generic<T>(x: T) {}`.
|
||||
if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind()
|
||||
&& arg_param.is_some()
|
||||
{
|
||||
*has_suggest_reborrow = true;
|
||||
self.suggest_reborrow(err, expr.span, moved_place);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let mut can_suggest_clone = true;
|
||||
if let Some(def_id) = def_id
|
||||
&& let Some(local_def_id) = def_id.as_local()
|
||||
&& let node = self.infcx.tcx.hir_node_by_def_id(local_def_id)
|
||||
&& let Some(fn_sig) = node.fn_sig()
|
||||
&& let Some(ident) = node.ident()
|
||||
&& let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
|
||||
&& let Some(arg) = fn_sig.decl.inputs.get(pos + offset)
|
||||
{
|
||||
let mut is_mut = false;
|
||||
if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = arg.kind
|
||||
&& let Res::Def(DefKind::TyParam, param_def_id) = path.res
|
||||
&& self
|
||||
.infcx
|
||||
.tcx
|
||||
.predicates_of(def_id)
|
||||
.instantiate_identity(self.infcx.tcx)
|
||||
.predicates
|
||||
.into_iter()
|
||||
.any(|pred| {
|
||||
if let ty::ClauseKind::Trait(predicate) = pred.kind().skip_binder()
|
||||
&& [
|
||||
self.infcx.tcx.get_diagnostic_item(sym::AsRef),
|
||||
self.infcx.tcx.get_diagnostic_item(sym::AsMut),
|
||||
self.infcx.tcx.get_diagnostic_item(sym::Borrow),
|
||||
self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
|
||||
]
|
||||
.contains(&Some(predicate.def_id()))
|
||||
&& let ty::Param(param) = predicate.self_ty().kind()
|
||||
&& let generics = self.infcx.tcx.generics_of(def_id)
|
||||
&& let param = generics.type_param(*param, self.infcx.tcx)
|
||||
&& param.def_id == param_def_id
|
||||
{
|
||||
if [
|
||||
self.infcx.tcx.get_diagnostic_item(sym::AsMut),
|
||||
self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
|
||||
]
|
||||
.contains(&Some(predicate.def_id()))
|
||||
{
|
||||
is_mut = true;
|
||||
}
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
})
|
||||
// If the moved place is used generically by the callee and a reference to it
|
||||
// would still satisfy any bounds on its type, suggest borrowing.
|
||||
if let Some(¶m) = arg_param
|
||||
&& let Some(generic_args) = call_id.and_then(|id| typeck.node_args_opt(id))
|
||||
&& let Some(ref_mutability) = self.suggest_borrow_generic_arg(
|
||||
err,
|
||||
def_id,
|
||||
generic_args,
|
||||
param,
|
||||
moved_place,
|
||||
pos + offset,
|
||||
ty,
|
||||
expr.span,
|
||||
)
|
||||
{
|
||||
// The type of the argument corresponding to the expression that got moved
|
||||
// is a type parameter `T`, which is has a `T: AsRef` obligation.
|
||||
err.span_suggestion_verbose(
|
||||
expr.span.shrink_to_lo(),
|
||||
"borrow the value to avoid moving it",
|
||||
format!("&{}", if is_mut { "mut " } else { "" }),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
can_suggest_clone = is_mut;
|
||||
} else {
|
||||
can_suggest_clone = ref_mutability.is_mut();
|
||||
} else if let Some(local_def_id) = def_id.as_local()
|
||||
&& let node = self.infcx.tcx.hir_node_by_def_id(local_def_id)
|
||||
&& let Some(fn_decl) = node.fn_decl()
|
||||
&& let Some(ident) = node.ident()
|
||||
&& let Some(arg) = fn_decl.inputs.get(pos + offset)
|
||||
{
|
||||
// If we can't suggest borrowing in the call, but the function definition
|
||||
// is local, instead offer changing the function to borrow that argument.
|
||||
let mut span: MultiSpan = arg.span.into();
|
||||
span.push_span_label(
|
||||
arg.span,
|
||||
@ -546,8 +511,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
);
|
||||
}
|
||||
}
|
||||
let place = &self.move_data.move_paths[mpi].place;
|
||||
let ty = place.ty(self.body, self.infcx.tcx).ty;
|
||||
if let hir::Node::Expr(parent_expr) = parent
|
||||
&& let hir::ExprKind::Call(call_expr, _) = parent_expr.kind
|
||||
&& let hir::ExprKind::Path(hir::QPath::LangItem(LangItem::IntoIterIntoIter, _)) =
|
||||
@ -557,6 +520,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
} else if let UseSpans::FnSelfUse { kind: CallKind::Normal { .. }, .. } = move_spans
|
||||
{
|
||||
// We already suggest cloning for these cases in `explain_captures`.
|
||||
} else if moved_or_invoked_closure {
|
||||
// Do not suggest `closure.clone()()`.
|
||||
} else if let UseSpans::ClosureUse {
|
||||
closure_kind:
|
||||
ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)),
|
||||
@ -665,6 +630,113 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
);
|
||||
}
|
||||
|
||||
/// If a place is used after being moved as an argument to a function, the function is generic
|
||||
/// in that argument, and a reference to the argument's type would still satisfy the function's
|
||||
/// bounds, suggest borrowing. This covers, e.g., borrowing an `impl Fn()` argument being passed
|
||||
/// in an `impl FnOnce()` position.
|
||||
/// Returns `Some(mutability)` when suggesting to borrow with mutability `mutability`, or `None`
|
||||
/// if no suggestion is made.
|
||||
fn suggest_borrow_generic_arg(
|
||||
&self,
|
||||
err: &mut Diag<'_>,
|
||||
callee_did: DefId,
|
||||
generic_args: ty::GenericArgsRef<'tcx>,
|
||||
param: ty::ParamTy,
|
||||
moved_place: PlaceRef<'tcx>,
|
||||
moved_arg_pos: usize,
|
||||
moved_arg_ty: Ty<'tcx>,
|
||||
place_span: Span,
|
||||
) -> Option<ty::Mutability> {
|
||||
let tcx = self.infcx.tcx;
|
||||
let sig = tcx.fn_sig(callee_did).instantiate_identity().skip_binder();
|
||||
let clauses = tcx.predicates_of(callee_did).instantiate_identity(self.infcx.tcx).predicates;
|
||||
|
||||
// First, is there at least one method on one of `param`'s trait bounds?
|
||||
// This keeps us from suggesting borrowing the argument to `mem::drop`, e.g.
|
||||
if !clauses.iter().any(|clause| {
|
||||
clause.as_trait_clause().is_some_and(|tc| {
|
||||
tc.self_ty().skip_binder().is_param(param.index)
|
||||
&& tc.polarity() == ty::PredicatePolarity::Positive
|
||||
&& tcx
|
||||
.supertrait_def_ids(tc.def_id())
|
||||
.flat_map(|trait_did| tcx.associated_items(trait_did).in_definition_order())
|
||||
.any(|item| item.fn_has_self_parameter)
|
||||
})
|
||||
}) {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Try borrowing a shared reference first, then mutably.
|
||||
if let Some(mutbl) = [ty::Mutability::Not, ty::Mutability::Mut].into_iter().find(|&mutbl| {
|
||||
let re = self.infcx.tcx.lifetimes.re_erased;
|
||||
let ref_ty = Ty::new_ref(self.infcx.tcx, re, moved_arg_ty, mutbl);
|
||||
|
||||
// Ensure that substituting `ref_ty` in the callee's signature doesn't break
|
||||
// other inputs or the return type.
|
||||
let new_args = tcx.mk_args_from_iter(generic_args.iter().enumerate().map(
|
||||
|(i, arg)| {
|
||||
if i == param.index as usize { ref_ty.into() } else { arg }
|
||||
},
|
||||
));
|
||||
let can_subst = |ty: Ty<'tcx>| {
|
||||
// Normalize before comparing to see through type aliases and projections.
|
||||
let old_ty = ty::EarlyBinder::bind(ty).instantiate(tcx, generic_args);
|
||||
let new_ty = ty::EarlyBinder::bind(ty).instantiate(tcx, new_args);
|
||||
if let Ok(old_ty) = tcx.try_normalize_erasing_regions(self.param_env, old_ty)
|
||||
&& let Ok(new_ty) = tcx.try_normalize_erasing_regions(self.param_env, new_ty)
|
||||
{
|
||||
old_ty == new_ty
|
||||
} else {
|
||||
false
|
||||
}
|
||||
};
|
||||
if !can_subst(sig.output())
|
||||
|| sig
|
||||
.inputs()
|
||||
.iter()
|
||||
.enumerate()
|
||||
.any(|(i, &input_ty)| i != moved_arg_pos && !can_subst(input_ty))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
// Test the callee's predicates, substituting a reference in for the self ty
|
||||
// in bounds on `param`.
|
||||
clauses.iter().all(|&clause| {
|
||||
let clause_for_ref = clause.kind().map_bound(|kind| match kind {
|
||||
ClauseKind::Trait(c) if c.self_ty().is_param(param.index) => {
|
||||
ClauseKind::Trait(c.with_self_ty(tcx, ref_ty))
|
||||
}
|
||||
ClauseKind::Projection(c) if c.self_ty().is_param(param.index) => {
|
||||
ClauseKind::Projection(c.with_self_ty(tcx, ref_ty))
|
||||
}
|
||||
_ => kind,
|
||||
});
|
||||
self.infcx.predicate_must_hold_modulo_regions(&Obligation::new(
|
||||
tcx,
|
||||
ObligationCause::dummy(),
|
||||
self.param_env,
|
||||
ty::EarlyBinder::bind(clause_for_ref).instantiate(tcx, generic_args),
|
||||
))
|
||||
})
|
||||
}) {
|
||||
let place_desc = if let Some(desc) = self.describe_place(moved_place) {
|
||||
format!("`{desc}`")
|
||||
} else {
|
||||
"here".to_owned()
|
||||
};
|
||||
err.span_suggestion_verbose(
|
||||
place_span.shrink_to_lo(),
|
||||
format!("consider {}borrowing {place_desc}", mutbl.mutably_str()),
|
||||
mutbl.ref_prefix_str(),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
Some(mutbl)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn report_use_of_uninitialized(
|
||||
&self,
|
||||
mpi: MovePathIndex,
|
||||
@ -845,74 +917,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
|
||||
);
|
||||
}
|
||||
|
||||
fn suggest_borrow_fn_like(
|
||||
&self,
|
||||
err: &mut Diag<'_>,
|
||||
ty: Ty<'tcx>,
|
||||
move_sites: &[MoveSite],
|
||||
value_name: &str,
|
||||
) -> bool {
|
||||
let tcx = self.infcx.tcx;
|
||||
|
||||
// Find out if the predicates show that the type is a Fn or FnMut
|
||||
let find_fn_kind_from_did = |(pred, _): (ty::Clause<'tcx>, _)| {
|
||||
if let ty::ClauseKind::Trait(pred) = pred.kind().skip_binder()
|
||||
&& pred.self_ty() == ty
|
||||
{
|
||||
if tcx.is_lang_item(pred.def_id(), LangItem::Fn) {
|
||||
return Some(hir::Mutability::Not);
|
||||
} else if tcx.is_lang_item(pred.def_id(), LangItem::FnMut) {
|
||||
return Some(hir::Mutability::Mut);
|
||||
}
|
||||
}
|
||||
None
|
||||
};
|
||||
|
||||
// If the type is opaque/param/closure, and it is Fn or FnMut, let's suggest (mutably)
|
||||
// borrowing the type, since `&mut F: FnMut` iff `F: FnMut` and similarly for `Fn`.
|
||||
// These types seem reasonably opaque enough that they could be instantiated with their
|
||||
// borrowed variants in a function body when we see a move error.
|
||||
let borrow_level = match *ty.kind() {
|
||||
ty::Param(_) => tcx
|
||||
.explicit_predicates_of(self.mir_def_id().to_def_id())
|
||||
.predicates
|
||||
.iter()
|
||||
.copied()
|
||||
.find_map(find_fn_kind_from_did),
|
||||
ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => tcx
|
||||
.explicit_item_super_predicates(def_id)
|
||||
.iter_instantiated_copied(tcx, args)
|
||||
.find_map(|(clause, span)| find_fn_kind_from_did((clause, span))),
|
||||
ty::Closure(_, args) => match args.as_closure().kind() {
|
||||
ty::ClosureKind::Fn => Some(hir::Mutability::Not),
|
||||
ty::ClosureKind::FnMut => Some(hir::Mutability::Mut),
|
||||
_ => None,
|
||||
},
|
||||
_ => None,
|
||||
};
|
||||
|
||||
let Some(borrow_level) = borrow_level else {
|
||||
return false;
|
||||
};
|
||||
let sugg = move_sites
|
||||
.iter()
|
||||
.map(|move_site| {
|
||||
let move_out = self.move_data.moves[(*move_site).moi];
|
||||
let moved_place = &self.move_data.move_paths[move_out.path].place;
|
||||
let move_spans = self.move_spans(moved_place.as_ref(), move_out.source);
|
||||
let move_span = move_spans.args_or_use();
|
||||
let suggestion = borrow_level.ref_prefix_str().to_owned();
|
||||
(move_span.shrink_to_lo(), suggestion)
|
||||
})
|
||||
.collect();
|
||||
err.multipart_suggestion_verbose(
|
||||
format!("consider {}borrowing {value_name}", borrow_level.mutably_str()),
|
||||
sugg,
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
true
|
||||
}
|
||||
|
||||
/// In a move error that occurs on a call within a loop, we try to identify cases where cloning
|
||||
/// the value would lead to a logic error. We infer these cases by seeing if the moved value is
|
||||
/// part of the logic to break the loop, either through an explicit `break` or if the expression
|
||||
|
@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
|
||||
|
|
||||
LL | if let MultiVariant::Point(ref mut x, _) = point {
|
||||
| ^^^^^
|
||||
help: consider mutably borrowing `c`
|
||||
|
|
||||
LL | let a = &mut c;
|
||||
| ++++
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
|
@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
|
||||
|
|
||||
LL | let SingleVariant::Point(ref mut x, _) = point;
|
||||
| ^^^^^
|
||||
help: consider mutably borrowing `c`
|
||||
|
|
||||
LL | let b = &mut c;
|
||||
| ++++
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
|
@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
|
||||
|
|
||||
LL | x.y.a += 1;
|
||||
| ^^^^^
|
||||
help: consider mutably borrowing `hello`
|
||||
|
|
||||
LL | let b = &mut hello;
|
||||
| ++++
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
|
@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
|
||||
|
|
||||
LL | x.0 += 1;
|
||||
| ^^^
|
||||
help: consider mutably borrowing `hello`
|
||||
|
|
||||
LL | let b = &mut hello;
|
||||
| ++++
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
|
@ -0,0 +1,20 @@
|
||||
//! auxiliary definitons for suggest-borrow-for-generic-arg.rs, to ensure the suggestion works on
|
||||
//! functions defined in other crates.
|
||||
|
||||
use std::io::{self, Read, Write};
|
||||
use std::iter::Sum;
|
||||
|
||||
pub fn write_stuff<W: Write>(mut writer: W) -> io::Result<()> {
|
||||
writeln!(writer, "stuff")
|
||||
}
|
||||
|
||||
pub fn read_and_discard<R: Read>(mut reader: R) -> io::Result<()> {
|
||||
let mut buf = Vec::new();
|
||||
reader.read_to_end(&mut buf).map(|_| ())
|
||||
}
|
||||
|
||||
pub fn sum_three<I: IntoIterator>(iter: I) -> <I as IntoIterator>::Item
|
||||
where <I as IntoIterator>::Item: Sum
|
||||
{
|
||||
iter.into_iter().take(3).sum()
|
||||
}
|
@ -1,4 +1,4 @@
|
||||
fn takes_fn(f: impl Fn()) { //~ HELP if `impl Fn()` implemented `Clone`
|
||||
fn takes_fn(f: impl Fn()) {
|
||||
loop {
|
||||
takes_fnonce(f);
|
||||
//~^ ERROR use of moved value
|
||||
|
@ -8,21 +8,6 @@ LL | loop {
|
||||
LL | takes_fnonce(f);
|
||||
| ^ value moved here, in previous iteration of loop
|
||||
|
|
||||
note: consider changing this parameter type in function `takes_fnonce` to borrow instead if owning the value isn't necessary
|
||||
--> $DIR/borrow-closures-instead-of-move.rs:34:20
|
||||
|
|
||||
LL | fn takes_fnonce(_: impl FnOnce()) {}
|
||||
| ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value
|
||||
| |
|
||||
| in this function
|
||||
help: if `impl Fn()` implemented `Clone`, you could clone the value
|
||||
--> $DIR/borrow-closures-instead-of-move.rs:1:16
|
||||
|
|
||||
LL | fn takes_fn(f: impl Fn()) {
|
||||
| ^^^^^^^^^ consider constraining this type parameter with `Clone`
|
||||
LL | loop {
|
||||
LL | takes_fnonce(f);
|
||||
| - you could clone this value
|
||||
help: consider borrowing `f`
|
||||
|
|
||||
LL | takes_fnonce(&f);
|
||||
@ -40,13 +25,6 @@ LL | takes_fnonce(m);
|
||||
LL | takes_fnonce(m);
|
||||
| ^ value used here after move
|
||||
|
|
||||
note: consider changing this parameter type in function `takes_fnonce` to borrow instead if owning the value isn't necessary
|
||||
--> $DIR/borrow-closures-instead-of-move.rs:34:20
|
||||
|
|
||||
LL | fn takes_fnonce(_: impl FnOnce()) {}
|
||||
| ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value
|
||||
| |
|
||||
| in this function
|
||||
help: if `impl FnMut()` implemented `Clone`, you could clone the value
|
||||
--> $DIR/borrow-closures-instead-of-move.rs:9:20
|
||||
|
|
||||
|
@ -8,7 +8,7 @@ LL | foo(bar);
|
||||
LL | let _baa = bar;
|
||||
| ^^^ value used here after move
|
||||
|
|
||||
help: borrow the value to avoid moving it
|
||||
help: consider borrowing `bar`
|
||||
|
|
||||
LL | foo(&bar);
|
||||
| +
|
||||
@ -31,7 +31,7 @@ LL | struct Bar;
|
||||
...
|
||||
LL | qux(bar);
|
||||
| --- you could clone this value
|
||||
help: borrow the value to avoid moving it
|
||||
help: consider mutably borrowing `bar`
|
||||
|
|
||||
LL | qux(&mut bar);
|
||||
| ++++
|
||||
@ -46,7 +46,7 @@ LL | bat(bar);
|
||||
LL | let _baa = bar;
|
||||
| ^^^ value used here after move
|
||||
|
|
||||
help: borrow the value to avoid moving it
|
||||
help: consider borrowing `bar`
|
||||
|
|
||||
LL | bat(&bar);
|
||||
| +
|
||||
@ -69,7 +69,7 @@ LL | struct Bar;
|
||||
...
|
||||
LL | baz(bar);
|
||||
| --- you could clone this value
|
||||
help: borrow the value to avoid moving it
|
||||
help: consider mutably borrowing `bar`
|
||||
|
|
||||
LL | baz(&mut bar);
|
||||
| ++++
|
||||
|
@ -24,10 +24,6 @@ LL | fn conspirator<F>(mut f: F) where F: FnMut(&mut R, bool) {
|
||||
| ^ consider constraining this type parameter with `Clone`
|
||||
LL | let mut r = R {c: Box::new(f)};
|
||||
| - you could clone this value
|
||||
help: consider mutably borrowing `f`
|
||||
|
|
||||
LL | let mut r = R {c: Box::new(&mut f)};
|
||||
| ++++
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
|
||||
|
46
tests/ui/moves/suggest-borrow-for-generic-arg.fixed
Normal file
46
tests/ui/moves/suggest-borrow-for-generic-arg.fixed
Normal file
@ -0,0 +1,46 @@
|
||||
//! Test suggetions to borrow generic arguments instead of moving. Tests for other instances of this
|
||||
//! can be found in `moved-value-on-as-ref-arg.rs` and `borrow-closures-instead-of-move.rs`
|
||||
//@ run-rustfix
|
||||
//@ aux-crate:aux=suggest-borrow-for-generic-arg-aux.rs
|
||||
//@ edition: 2021
|
||||
|
||||
#![allow(unused_mut)]
|
||||
use std::io::{self, Write};
|
||||
|
||||
// test for `std::io::Write` (#131413)
|
||||
fn test_write() -> io::Result<()> {
|
||||
let mut stdout = io::stdout();
|
||||
aux::write_stuff(&stdout)?; //~ HELP consider borrowing `stdout`
|
||||
writeln!(stdout, "second line")?; //~ ERROR borrow of moved value: `stdout`
|
||||
|
||||
let mut buf = Vec::new();
|
||||
aux::write_stuff(&mut buf.clone())?; //~ HELP consider mutably borrowing `buf`
|
||||
//~^ HELP consider cloning the value
|
||||
writeln!(buf, "second_line") //~ ERROR borrow of moved value: `buf`
|
||||
}
|
||||
|
||||
/// test for `std::io::Read` (#131413)
|
||||
fn test_read() -> io::Result<()> {
|
||||
let stdin = io::stdin();
|
||||
aux::read_and_discard(&stdin)?; //~ HELP consider borrowing `stdin`
|
||||
aux::read_and_discard(stdin)?; //~ ERROR use of moved value: `stdin`
|
||||
|
||||
let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]);
|
||||
aux::read_and_discard(&mut bytes.clone())?; //~ HELP consider mutably borrowing `bytes`
|
||||
//~^ HELP consider cloning the value
|
||||
aux::read_and_discard(bytes) //~ ERROR use of moved value: `bytes`
|
||||
}
|
||||
|
||||
/// test that suggestions work with projection types in the callee's signature
|
||||
fn test_projections() {
|
||||
let mut iter = [1, 2, 3, 4, 5, 6].into_iter();
|
||||
let _six: usize = aux::sum_three(&mut iter.clone()); //~ HELP consider mutably borrowing `iter`
|
||||
//~^ HELP consider cloning the value
|
||||
let _fifteen: usize = aux::sum_three(iter); //~ ERROR use of moved value: `iter`
|
||||
}
|
||||
|
||||
fn main() {
|
||||
test_write().unwrap();
|
||||
test_read().unwrap();
|
||||
test_projections();
|
||||
}
|
46
tests/ui/moves/suggest-borrow-for-generic-arg.rs
Normal file
46
tests/ui/moves/suggest-borrow-for-generic-arg.rs
Normal file
@ -0,0 +1,46 @@
|
||||
//! Test suggetions to borrow generic arguments instead of moving. Tests for other instances of this
|
||||
//! can be found in `moved-value-on-as-ref-arg.rs` and `borrow-closures-instead-of-move.rs`
|
||||
//@ run-rustfix
|
||||
//@ aux-crate:aux=suggest-borrow-for-generic-arg-aux.rs
|
||||
//@ edition: 2021
|
||||
|
||||
#![allow(unused_mut)]
|
||||
use std::io::{self, Write};
|
||||
|
||||
// test for `std::io::Write` (#131413)
|
||||
fn test_write() -> io::Result<()> {
|
||||
let mut stdout = io::stdout();
|
||||
aux::write_stuff(stdout)?; //~ HELP consider borrowing `stdout`
|
||||
writeln!(stdout, "second line")?; //~ ERROR borrow of moved value: `stdout`
|
||||
|
||||
let mut buf = Vec::new();
|
||||
aux::write_stuff(buf)?; //~ HELP consider mutably borrowing `buf`
|
||||
//~^ HELP consider cloning the value
|
||||
writeln!(buf, "second_line") //~ ERROR borrow of moved value: `buf`
|
||||
}
|
||||
|
||||
/// test for `std::io::Read` (#131413)
|
||||
fn test_read() -> io::Result<()> {
|
||||
let stdin = io::stdin();
|
||||
aux::read_and_discard(stdin)?; //~ HELP consider borrowing `stdin`
|
||||
aux::read_and_discard(stdin)?; //~ ERROR use of moved value: `stdin`
|
||||
|
||||
let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]);
|
||||
aux::read_and_discard(bytes)?; //~ HELP consider mutably borrowing `bytes`
|
||||
//~^ HELP consider cloning the value
|
||||
aux::read_and_discard(bytes) //~ ERROR use of moved value: `bytes`
|
||||
}
|
||||
|
||||
/// test that suggestions work with projection types in the callee's signature
|
||||
fn test_projections() {
|
||||
let mut iter = [1, 2, 3, 4, 5, 6].into_iter();
|
||||
let _six: usize = aux::sum_three(iter); //~ HELP consider mutably borrowing `iter`
|
||||
//~^ HELP consider cloning the value
|
||||
let _fifteen: usize = aux::sum_three(iter); //~ ERROR use of moved value: `iter`
|
||||
}
|
||||
|
||||
fn main() {
|
||||
test_write().unwrap();
|
||||
test_read().unwrap();
|
||||
test_projections();
|
||||
}
|
93
tests/ui/moves/suggest-borrow-for-generic-arg.stderr
Normal file
93
tests/ui/moves/suggest-borrow-for-generic-arg.stderr
Normal file
@ -0,0 +1,93 @@
|
||||
error[E0382]: borrow of moved value: `stdout`
|
||||
--> $DIR/suggest-borrow-for-generic-arg.rs:14:14
|
||||
|
|
||||
LL | let mut stdout = io::stdout();
|
||||
| ---------- move occurs because `stdout` has type `Stdout`, which does not implement the `Copy` trait
|
||||
LL | aux::write_stuff(stdout)?;
|
||||
| ------ value moved here
|
||||
LL | writeln!(stdout, "second line")?;
|
||||
| ^^^^^^ value borrowed here after move
|
||||
|
|
||||
help: consider borrowing `stdout`
|
||||
|
|
||||
LL | aux::write_stuff(&stdout)?;
|
||||
| +
|
||||
|
||||
error[E0382]: borrow of moved value: `buf`
|
||||
--> $DIR/suggest-borrow-for-generic-arg.rs:19:14
|
||||
|
|
||||
LL | let mut buf = Vec::new();
|
||||
| ------- move occurs because `buf` has type `Vec<u8>`, which does not implement the `Copy` trait
|
||||
LL | aux::write_stuff(buf)?;
|
||||
| --- value moved here
|
||||
LL |
|
||||
LL | writeln!(buf, "second_line")
|
||||
| ^^^ value borrowed here after move
|
||||
|
|
||||
help: consider mutably borrowing `buf`
|
||||
|
|
||||
LL | aux::write_stuff(&mut buf)?;
|
||||
| ++++
|
||||
help: consider cloning the value if the performance cost is acceptable
|
||||
|
|
||||
LL | aux::write_stuff(buf.clone())?;
|
||||
| ++++++++
|
||||
|
||||
error[E0382]: use of moved value: `stdin`
|
||||
--> $DIR/suggest-borrow-for-generic-arg.rs:26:27
|
||||
|
|
||||
LL | let stdin = io::stdin();
|
||||
| ----- move occurs because `stdin` has type `Stdin`, which does not implement the `Copy` trait
|
||||
LL | aux::read_and_discard(stdin)?;
|
||||
| ----- value moved here
|
||||
LL | aux::read_and_discard(stdin)?;
|
||||
| ^^^^^ value used here after move
|
||||
|
|
||||
help: consider borrowing `stdin`
|
||||
|
|
||||
LL | aux::read_and_discard(&stdin)?;
|
||||
| +
|
||||
|
||||
error[E0382]: use of moved value: `bytes`
|
||||
--> $DIR/suggest-borrow-for-generic-arg.rs:31:27
|
||||
|
|
||||
LL | let mut bytes = std::collections::VecDeque::from([1, 2, 3, 4, 5, 6]);
|
||||
| --------- move occurs because `bytes` has type `VecDeque<u8>`, which does not implement the `Copy` trait
|
||||
LL | aux::read_and_discard(bytes)?;
|
||||
| ----- value moved here
|
||||
LL |
|
||||
LL | aux::read_and_discard(bytes)
|
||||
| ^^^^^ value used here after move
|
||||
|
|
||||
help: consider mutably borrowing `bytes`
|
||||
|
|
||||
LL | aux::read_and_discard(&mut bytes)?;
|
||||
| ++++
|
||||
help: consider cloning the value if the performance cost is acceptable
|
||||
|
|
||||
LL | aux::read_and_discard(bytes.clone())?;
|
||||
| ++++++++
|
||||
|
||||
error[E0382]: use of moved value: `iter`
|
||||
--> $DIR/suggest-borrow-for-generic-arg.rs:39:42
|
||||
|
|
||||
LL | let mut iter = [1, 2, 3, 4, 5, 6].into_iter();
|
||||
| -------- move occurs because `iter` has type `std::array::IntoIter<usize, 6>`, which does not implement the `Copy` trait
|
||||
LL | let _six: usize = aux::sum_three(iter);
|
||||
| ---- value moved here
|
||||
LL |
|
||||
LL | let _fifteen: usize = aux::sum_three(iter);
|
||||
| ^^^^ value used here after move
|
||||
|
|
||||
help: consider mutably borrowing `iter`
|
||||
|
|
||||
LL | let _six: usize = aux::sum_three(&mut iter);
|
||||
| ++++
|
||||
help: consider cloning the value if the performance cost is acceptable
|
||||
|
|
||||
LL | let _six: usize = aux::sum_three(iter.clone());
|
||||
| ++++++++
|
||||
|
||||
error: aborting due to 5 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0382`.
|
@ -11,10 +11,6 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t
|
||||
|
|
||||
LL | a += 1;
|
||||
| ^
|
||||
help: consider mutably borrowing `hello`
|
||||
|
|
||||
LL | let b = &mut hello;
|
||||
| ++++
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user