Skip to content

Commit f67523d

Browse files
borsbors
committed
Auto merge of rust-lang#118076 - estebank:issue-109429, r=davidtwco
Tweak `.clone()` suggestion to work in more cases When going through auto-deref, the `<T as Clone>` impl sometimes needs to be specified for rustc to actually clone the value and not the reference. ``` error[E0507]: cannot move out of dereference of `S` --> $DIR/needs-clone-through-deref.rs:15:18 | LL | for _ in self.clone().into_iter() {} | ^^^^^^^^^^^^ ----------- value moved due to this method call | | | move occurs because value has type `Vec<usize>`, which does not implement the `Copy` trait | note: `into_iter` takes ownership of the receiver `self`, which moves value --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL help: you can `clone` the value and consume it, but this might not be your desired behavior | LL | for _ in <Vec<usize> as Clone>::clone(&self.clone()).into_iter() {} | ++++++++++++++++++++++++++++++ + ``` When encountering a move error, look for implementations of `Clone` for the moved type. If there is one, check if all its obligations are met. If they are, we suggest cloning without caveats. If they aren't, we suggest cloning while mentioning the unmet obligations, potentially suggesting `#[derive(Clone)]` when appropriate. ``` error[E0507]: cannot move out of a shared reference --> $DIR/suggest-clone-when-some-obligation-is-unmet.rs:20:28 | LL | let mut copy: Vec<U> = map.clone().into_values().collect(); | ^^^^^^^^^^^ ------------- value moved due to this method call | | | move occurs because value has type `HashMap<T, U, Hash128_1>`, which does not implement the `Copy` trait | note: `HashMap::<K, V, S>::into_values` takes ownership of the receiver `self`, which moves value --> $SRC_DIR/std/src/collections/hash/map.rs:LL:COL help: you could `clone` the value and consume it, if the `Hash128_1: Clone` trait bound could be satisfied | LL | let mut copy: Vec<U> = <HashMap<T, U, Hash128_1> as Clone>::clone(&map.clone()).into_values().collect(); | ++++++++++++++++++++++++++++++++++++++++++++ + help: consider annotating `Hash128_1` with `#[derive(Clone)]` | LL + #[derive(Clone)] LL | pub struct Hash128_1; | ``` Fix rust-lang#109429. When encountering multiple mutable borrows, suggest cloning and adding derive annotations as needed. ``` error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference --> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9 | LL | foo(&mut sm.x); | ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable | help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str` --> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21 | LL | let mut sm = sr.clone(); | ^^^^^^^ help: consider annotating `Str` with `#[derive(Clone)]` | LL + #[derive(Clone)] LL | struct Str { | help: consider specifying this binding's type | LL | let mut sm: &mut Str = sr.clone(); | ++++++++++ ``` Fix rust-lang#34629. Fix rust-lang#76643. Fix rust-lang#91532.
2 parents 9358642 + a8faa82 commit f67523d

39 files changed

+800
-97
lines changed

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

+103-52
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rustc_hir::def::{CtorKind, Namespace};
1111
use rustc_hir::CoroutineKind;
1212
use rustc_index::IndexSlice;
1313
use rustc_infer::infer::BoundRegionConversionTime;
14+
use rustc_infer::traits::{FulfillmentErrorCode, SelectionError};
1415
use rustc_middle::mir::tcx::PlaceTy;
1516
use rustc_middle::mir::{
1617
AggregateKind, CallSource, ConstOperand, FakeReadCause, Local, LocalInfo, LocalKind, Location,
@@ -24,10 +25,9 @@ use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
2425
use rustc_span::def_id::LocalDefId;
2526
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
2627
use rustc_target::abi::{FieldIdx, VariantIdx};
27-
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
28-
use rustc_trait_selection::traits::{
29-
type_known_to_meet_bound_modulo_regions, Obligation, ObligationCause,
30-
};
28+
use rustc_trait_selection::infer::InferCtxtExt;
29+
use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt as _;
30+
use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;
3131

3232
use super::borrow_set::BorrowData;
3333
use super::MirBorrowckCtxt;
@@ -1043,7 +1043,38 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10431043
}
10441044
CallKind::Normal { self_arg, desugaring, method_did, method_args } => {
10451045
let self_arg = self_arg.unwrap();
1046+
let mut has_sugg = false;
10461047
let tcx = self.infcx.tcx;
1048+
// Avoid pointing to the same function in multiple different
1049+
// error messages.
1050+
if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
1051+
self.explain_iterator_advancement_in_for_loop_if_applicable(
1052+
err,
1053+
span,
1054+
&move_spans,
1055+
);
1056+
1057+
let func = tcx.def_path_str(method_did);
1058+
err.subdiagnostic(CaptureReasonNote::FuncTakeSelf {
1059+
func,
1060+
place_name: place_name.clone(),
1061+
span: self_arg.span,
1062+
});
1063+
}
1064+
let parent_did = tcx.parent(method_did);
1065+
let parent_self_ty =
1066+
matches!(tcx.def_kind(parent_did), rustc_hir::def::DefKind::Impl { .. })
1067+
.then_some(parent_did)
1068+
.and_then(|did| match tcx.type_of(did).instantiate_identity().kind() {
1069+
ty::Adt(def, ..) => Some(def.did()),
1070+
_ => None,
1071+
});
1072+
let is_option_or_result = parent_self_ty.is_some_and(|def_id| {
1073+
matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result))
1074+
});
1075+
if is_option_or_result && maybe_reinitialized_locations_is_empty {
1076+
err.subdiagnostic(CaptureReasonLabel::BorrowContent { var_span });
1077+
}
10471078
if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
10481079
let ty = moved_place.ty(self.body, tcx).ty;
10491080
let suggest = match tcx.get_diagnostic_item(sym::IntoIterator) {
@@ -1108,7 +1139,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
11081139
// Erase and shadow everything that could be passed to the new infcx.
11091140
let ty = moved_place.ty(self.body, tcx).ty;
11101141

1111-
if let ty::Adt(def, args) = ty.kind()
1142+
if let ty::Adt(def, args) = ty.peel_refs().kind()
11121143
&& Some(def.did()) == tcx.lang_items().pin_type()
11131144
&& let ty::Ref(_, _, hir::Mutability::Mut) = args.type_at(0).kind()
11141145
&& let self_ty = self.infcx.instantiate_binder_with_fresh_vars(
@@ -1124,56 +1155,76 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
11241155
span: move_span.shrink_to_hi(),
11251156
},
11261157
);
1158+
has_sugg = true;
11271159
}
1128-
if let Some(clone_trait) = tcx.lang_items().clone_trait()
1129-
&& let trait_ref = ty::TraitRef::new(tcx, clone_trait, [ty])
1130-
&& let o = Obligation::new(
1131-
tcx,
1132-
ObligationCause::dummy(),
1133-
self.param_env,
1134-
ty::Binder::dummy(trait_ref),
1135-
)
1136-
&& self.infcx.predicate_must_hold_modulo_regions(&o)
1137-
{
1138-
err.span_suggestion_verbose(
1139-
move_span.shrink_to_hi(),
1140-
"you can `clone` the value and consume it, but this might not be \
1141-
your desired behavior",
1142-
".clone()".to_string(),
1143-
Applicability::MaybeIncorrect,
1144-
);
1160+
if let Some(clone_trait) = tcx.lang_items().clone_trait() {
1161+
let sugg = if moved_place
1162+
.iter_projections()
1163+
.any(|(_, elem)| matches!(elem, ProjectionElem::Deref))
1164+
{
1165+
vec![
1166+
// We use the fully-qualified path because `.clone()` can
1167+
// sometimes choose `<&T as Clone>` instead of `<T as Clone>`
1168+
// when going through auto-deref, so this ensures that doesn't
1169+
// happen, causing suggestions for `.clone().clone()`.
1170+
(move_span.shrink_to_lo(), format!("<{ty} as Clone>::clone(&")),
1171+
(move_span.shrink_to_hi(), ")".to_string()),
1172+
]
1173+
} else {
1174+
vec![(move_span.shrink_to_hi(), ".clone()".to_string())]
1175+
};
1176+
if let Some(errors) =
1177+
self.infcx.could_impl_trait(clone_trait, ty, self.param_env)
1178+
&& !has_sugg
1179+
{
1180+
let msg = match &errors[..] {
1181+
[] => "you can `clone` the value and consume it, but this \
1182+
might not be your desired behavior"
1183+
.to_string(),
1184+
[error] => {
1185+
format!(
1186+
"you could `clone` the value and consume it, if \
1187+
the `{}` trait bound could be satisfied",
1188+
error.obligation.predicate,
1189+
)
1190+
}
1191+
[errors @ .., last] => {
1192+
format!(
1193+
"you could `clone` the value and consume it, if \
1194+
the following trait bounds could be satisfied: {} \
1195+
and `{}`",
1196+
errors
1197+
.iter()
1198+
.map(|e| format!("`{}`", e.obligation.predicate))
1199+
.collect::<Vec<_>>()
1200+
.join(", "),
1201+
last.obligation.predicate,
1202+
)
1203+
}
1204+
};
1205+
err.multipart_suggestion_verbose(
1206+
msg,
1207+
sugg.clone(),
1208+
Applicability::MaybeIncorrect,
1209+
);
1210+
for error in errors {
1211+
if let FulfillmentErrorCode::CodeSelectionError(
1212+
SelectionError::Unimplemented,
1213+
) = error.code
1214+
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
1215+
pred,
1216+
)) = error.obligation.predicate.kind().skip_binder()
1217+
{
1218+
self.infcx.err_ctxt().suggest_derive(
1219+
&error.obligation,
1220+
err,
1221+
error.obligation.predicate.kind().rebind(pred),
1222+
);
1223+
}
1224+
}
1225+
}
11451226
}
11461227
}
1147-
// Avoid pointing to the same function in multiple different
1148-
// error messages.
1149-
if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
1150-
self.explain_iterator_advancement_in_for_loop_if_applicable(
1151-
err,
1152-
span,
1153-
&move_spans,
1154-
);
1155-
1156-
let func = tcx.def_path_str(method_did);
1157-
err.subdiagnostic(CaptureReasonNote::FuncTakeSelf {
1158-
func,
1159-
place_name,
1160-
span: self_arg.span,
1161-
});
1162-
}
1163-
let parent_did = tcx.parent(method_did);
1164-
let parent_self_ty =
1165-
matches!(tcx.def_kind(parent_did), rustc_hir::def::DefKind::Impl { .. })
1166-
.then_some(parent_did)
1167-
.and_then(|did| match tcx.type_of(did).instantiate_identity().kind() {
1168-
ty::Adt(def, ..) => Some(def.did()),
1169-
_ => None,
1170-
});
1171-
let is_option_or_result = parent_self_ty.is_some_and(|def_id| {
1172-
matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result))
1173-
});
1174-
if is_option_or_result && maybe_reinitialized_locations_is_empty {
1175-
err.subdiagnostic(CaptureReasonLabel::BorrowContent { var_span });
1176-
}
11771228
}
11781229
// Other desugarings takes &self, which cannot cause a move
11791230
_ => {}

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

+101-1
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
33
use rustc_hir as hir;
44
use rustc_hir::intravisit::Visitor;
55
use rustc_hir::Node;
6+
use rustc_infer::traits;
67
use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
7-
use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt};
8+
use rustc_middle::ty::{self, InstanceDef, ToPredicate, Ty, TyCtxt};
89
use rustc_middle::{
910
hir::place::PlaceBase,
1011
mir::{self, BindingForm, Local, LocalDecl, LocalInfo, LocalKind, Location},
1112
};
1213
use rustc_span::symbol::{kw, Symbol};
1314
use rustc_span::{sym, BytePos, DesugaringKind, Span};
1415
use rustc_target::abi::FieldIdx;
16+
use rustc_trait_selection::infer::InferCtxtExt;
17+
use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt;
1518

1619
use crate::diagnostics::BorrowedContentSource;
1720
use crate::util::FindAssignments;
@@ -1212,6 +1215,103 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
12121215
if let Some(hir_id) = hir_id
12131216
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
12141217
{
1218+
let tables = self.infcx.tcx.typeck(def_id.as_local().unwrap());
1219+
if let Some(clone_trait) = self.infcx.tcx.lang_items().clone_trait()
1220+
&& let Some(expr) = local.init
1221+
&& let ty = tables.node_type_opt(expr.hir_id)
1222+
&& let Some(ty) = ty
1223+
&& let ty::Ref(..) = ty.kind()
1224+
{
1225+
match self
1226+
.infcx
1227+
.could_impl_trait(clone_trait, ty.peel_refs(), self.param_env)
1228+
.as_deref()
1229+
{
1230+
Some([]) => {
1231+
// The type implements Clone.
1232+
err.span_help(
1233+
expr.span,
1234+
format!(
1235+
"you can `clone` the `{}` value and consume it, but this \
1236+
might not be your desired behavior",
1237+
ty.peel_refs(),
1238+
),
1239+
);
1240+
}
1241+
None => {
1242+
if let hir::ExprKind::MethodCall(segment, _rcvr, [], span) =
1243+
expr.kind
1244+
&& segment.ident.name == sym::clone
1245+
{
1246+
err.span_help(
1247+
span,
1248+
format!(
1249+
"`{}` doesn't implement `Clone`, so this call clones \
1250+
the reference `{ty}`",
1251+
ty.peel_refs(),
1252+
),
1253+
);
1254+
}
1255+
// The type doesn't implement Clone.
1256+
let trait_ref = ty::Binder::dummy(ty::TraitRef::new(
1257+
self.infcx.tcx,
1258+
clone_trait,
1259+
[ty.peel_refs()],
1260+
));
1261+
let obligation = traits::Obligation::new(
1262+
self.infcx.tcx,
1263+
traits::ObligationCause::dummy(),
1264+
self.param_env,
1265+
trait_ref,
1266+
);
1267+
self.infcx.err_ctxt().suggest_derive(
1268+
&obligation,
1269+
err,
1270+
trait_ref.to_predicate(self.infcx.tcx),
1271+
);
1272+
}
1273+
Some(errors) => {
1274+
if let hir::ExprKind::MethodCall(segment, _rcvr, [], span) =
1275+
expr.kind
1276+
&& segment.ident.name == sym::clone
1277+
{
1278+
err.span_help(
1279+
span,
1280+
format!(
1281+
"`{}` doesn't implement `Clone` because its \
1282+
implementations trait bounds could not be met, so \
1283+
this call clones the reference `{ty}`",
1284+
ty.peel_refs(),
1285+
),
1286+
);
1287+
err.note(format!(
1288+
"the following trait bounds weren't met: {}",
1289+
errors
1290+
.iter()
1291+
.map(|e| e.obligation.predicate.to_string())
1292+
.collect::<Vec<_>>()
1293+
.join("\n"),
1294+
));
1295+
}
1296+
// The type doesn't implement Clone because of unmet obligations.
1297+
for error in errors {
1298+
if let traits::FulfillmentErrorCode::CodeSelectionError(
1299+
traits::SelectionError::Unimplemented,
1300+
) = error.code
1301+
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
1302+
pred,
1303+
)) = error.obligation.predicate.kind().skip_binder()
1304+
{
1305+
self.infcx.err_ctxt().suggest_derive(
1306+
&error.obligation,
1307+
err,
1308+
error.obligation.predicate.kind().rebind(pred),
1309+
);
1310+
}
1311+
}
1312+
}
1313+
}
1314+
}
12151315
let (changing, span, sugg) = match local.ty {
12161316
Some(ty) => ("changing", ty.span, message),
12171317
None => {

0 commit comments

Comments
 (0)