Skip to content

Commit 086bbd2

Browse files
committed
Account for move error in the spread operator on struct literals
We attempt to suggest an appropriate clone for move errors on expressions like `S { ..s }` where a field isn't `Copy`. If we can't suggest, we still don't emit the incorrect suggestion of `S { ..s }.clone()`. ``` error[E0509]: cannot move out of type `S<K>`, which implements the `Drop` trait --> $DIR/borrowck-struct-update-with-dtor.rs:28:19 | LL | let _s2 = S { a: 2, ..s0 }; | ^^^^^^^^^^^^^^^^ | | | cannot move out of here | move occurs because `s0.c` has type `K`, which does not implement the `Copy` trait | help: clone the value from the field instead of using the spread operator syntax | LL | let _s2 = S { a: 2, c: s0.c.clone(), ..s0 }; | +++++++++++++++++ ``` ``` error[E0509]: cannot move out of type `S<()>`, which implements the `Drop` trait --> $DIR/borrowck-struct-update-with-dtor.rs:20:19 | LL | let _s2 = S { a: 2, ..s0 }; | ^^^^^^^^^^^^^^^^ | | | cannot move out of here | move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait | note: `B` doesn't implement `Copy` or `Clone` --> $DIR/borrowck-struct-update-with-dtor.rs:4:1 | LL | struct B; | ^^^^^^^^ help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax | LL | let _s2 = S { a: 2, b: s0.b.clone(), ..s0 }; | +++++++++++++++++ ```
1 parent c1f8bef commit 086bbd2

File tree

4 files changed

+343
-39
lines changed

4 files changed

+343
-39
lines changed

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 107 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -989,13 +989,109 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
989989
can_suggest_clone
990990
}
991991

992+
fn suggest_cloning_on_spread_operator(
993+
&self,
994+
err: &mut Diag<'_>,
995+
ty: Ty<'tcx>,
996+
expr: &'cx hir::Expr<'cx>,
997+
) {
998+
let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
999+
let hir::ExprKind::Struct(struct_qpath, fields, Some(base)) = expr.kind else { return };
1000+
let hir::QPath::Resolved(_, path) = struct_qpath else { return };
1001+
let hir::def::Res::Def(_, def_id) = path.res else { return };
1002+
let Some(expr_ty) = typeck_results.node_type_opt(expr.hir_id) else { return };
1003+
let ty::Adt(def, args) = expr_ty.kind() else { return };
1004+
let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = base.kind else { return };
1005+
let (hir::def::Res::Local(_)
1006+
| hir::def::Res::Def(
1007+
DefKind::Const | DefKind::ConstParam | DefKind::Static { .. } | DefKind::AssocConst,
1008+
_,
1009+
)) = path.res
1010+
else {
1011+
return;
1012+
};
1013+
let Ok(base_str) = self.infcx.tcx.sess.source_map().span_to_snippet(base.span) else {
1014+
return;
1015+
};
1016+
1017+
// 1. look for the fields of type `ty`.
1018+
// 2. check if they are clone and add them to suggestion
1019+
// 3. check if there are any values left to `..` and remove it if not
1020+
// 4. emit suggestion to clone the field directly as `bar: base.bar.clone()`
1021+
1022+
let mut final_field_count = fields.len();
1023+
let Some(variant) = def.variants().iter().find(|variant| variant.def_id == def_id) else {
1024+
// When we have an enum, look for the variant that corresponds to the variant the user
1025+
// wrote.
1026+
return;
1027+
};
1028+
let mut sugg = vec![];
1029+
for field in &variant.fields {
1030+
// In practice unless there are more than one field with the same type, we'll be
1031+
// suggesting a single field at a type, because we don't aggregate multiple borrow
1032+
// checker errors involving the spread operator into a single one.
1033+
let field_ty = field.ty(self.infcx.tcx, args);
1034+
let ident = field.ident(self.infcx.tcx);
1035+
if field_ty == ty && fields.iter().all(|field| field.ident.name != ident.name) {
1036+
// Suggest adding field and cloning it.
1037+
sugg.push(format!("{ident}: {base_str}.{ident}.clone()"));
1038+
final_field_count += 1;
1039+
}
1040+
}
1041+
let (span, sugg) = match fields {
1042+
[.., last] => (
1043+
if final_field_count == variant.fields.len() {
1044+
// We'll remove the `..base` as there aren't any fields left.
1045+
last.span.shrink_to_hi().with_hi(base.span.hi())
1046+
} else {
1047+
last.span.shrink_to_hi()
1048+
},
1049+
format!(", {}", sugg.join(", ")),
1050+
),
1051+
// Account for no fields in suggestion span.
1052+
[] => (
1053+
expr.span.with_lo(struct_qpath.span().hi()),
1054+
if final_field_count == variant.fields.len() {
1055+
// We'll remove the `..base` as there aren't any fields left.
1056+
format!(" {{ {} }}", sugg.join(", "))
1057+
} else {
1058+
format!(" {{ {}, ..{base_str} }}", sugg.join(", "))
1059+
},
1060+
),
1061+
};
1062+
let prefix = if !self.implements_clone(ty) {
1063+
let msg = format!("`{ty}` doesn't implement `Copy` or `Clone`");
1064+
if let ty::Adt(def, _) = ty.kind() {
1065+
err.span_note(self.infcx.tcx.def_span(def.did()), msg);
1066+
} else {
1067+
err.note(msg);
1068+
}
1069+
format!("if `{ty}` implemented `Clone`, you could ")
1070+
} else {
1071+
String::new()
1072+
};
1073+
let msg = format!(
1074+
"{prefix}clone the value from the field instead of using the spread operator syntax",
1075+
);
1076+
err.span_suggestion_verbose(span, msg, sugg, Applicability::MachineApplicable);
1077+
}
1078+
9921079
pub(crate) fn suggest_cloning(
9931080
&self,
9941081
err: &mut Diag<'_>,
9951082
ty: Ty<'tcx>,
9961083
mut expr: &'cx hir::Expr<'cx>,
9971084
mut other_expr: Option<&'cx hir::Expr<'cx>>,
9981085
) {
1086+
if let hir::ExprKind::Struct(_, _, Some(_)) = expr.kind {
1087+
// We have `S { foo: val, ..base }`. In `check_aggregate_rvalue` we have a single
1088+
// `Location` that covers both the `S { ... }` literal, all of its fields and the
1089+
// `base`. If the move happens because of `S { foo: val, bar: base.bar }` the `expr`
1090+
// will already be correct. Instead, we see if we can suggest writing.
1091+
self.suggest_cloning_on_spread_operator(err, ty, expr);
1092+
return;
1093+
}
1094+
9991095
if let Some(some_other_expr) = other_expr
10001096
&& let Some(parent_binop) =
10011097
self.infcx.tcx.hir().parent_iter(expr.hir_id).find_map(|n| {
@@ -1077,11 +1173,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10771173
} else {
10781174
false
10791175
})
1080-
&& let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait()
1081-
&& self
1082-
.infcx
1083-
.type_implements_trait(clone_trait_def, [call_ty], self.param_env)
1084-
.must_apply_modulo_regions()
1176+
&& self.implements_clone(call_ty)
10851177
&& self.suggest_cloning_inner(err, call_ty, parent)
10861178
{
10871179
return;
@@ -1091,16 +1183,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
10911183
}
10921184
}
10931185
let ty = ty.peel_refs();
1094-
if let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait()
1095-
&& self
1096-
.infcx
1097-
.type_implements_trait(clone_trait_def, [ty], self.param_env)
1098-
.must_apply_modulo_regions()
1099-
{
1186+
if self.implements_clone(ty) {
11001187
self.suggest_cloning_inner(err, ty, expr);
1188+
// } else {
1189+
// err.note(format!("if `{ty}` implemented `Clone`, you could clone the value"));
11011190
}
11021191
}
11031192

1193+
fn implements_clone(&self, ty: Ty<'tcx>) -> bool {
1194+
let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait() else { return false };
1195+
self.infcx
1196+
.type_implements_trait(clone_trait_def, [ty], self.param_env)
1197+
.must_apply_modulo_regions()
1198+
}
1199+
11041200
pub(crate) fn clone_on_reference(&self, expr: &hir::Expr<'_>) -> Option<Span> {
11051201
let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
11061202
if let hir::ExprKind::MethodCall(segment, rcvr, args, span) = expr.kind

tests/ui/borrowck/borrowck-struct-update-with-dtor.rs

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,63 @@
22
// move, when the struct implements Drop.
33

44
struct B;
5-
struct S { a: isize, b: B }
6-
impl Drop for S { fn drop(&mut self) { } }
5+
struct S<K> { a: isize, b: B, c: K }
6+
impl<K> Drop for S<K> { fn drop(&mut self) { } }
77

8-
struct T { a: isize, mv: Box<isize> }
8+
struct T { a: isize, b: Box<isize> }
99
impl Drop for T { fn drop(&mut self) { } }
1010

11-
fn f(s0:S) {
12-
let _s2 = S{a: 2, ..s0};
13-
//~^ ERROR [E0509]
11+
struct V<K> { a: isize, b: Box<isize>, c: K }
12+
impl<K> Drop for V<K> { fn drop(&mut self) { } }
13+
14+
#[derive(Clone)]
15+
struct Clonable;
16+
17+
mod not_all_clone {
18+
use super::*;
19+
fn a(s0: S<()>) {
20+
let _s2 = S { a: 2, ..s0 };
21+
//~^ ERROR [E0509]
22+
}
23+
fn b(s0: S<B>) {
24+
let _s2 = S { a: 2, ..s0 };
25+
//~^ ERROR [E0509]
26+
//~| ERROR [E0509]
27+
}
28+
fn c<K: Clone>(s0: S<K>) {
29+
let _s2 = S { a: 2, ..s0 };
30+
//~^ ERROR [E0509]
31+
//~| ERROR [E0509]
32+
}
1433
}
34+
mod all_clone {
35+
use super::*;
36+
fn a(s0: T) {
37+
let _s2 = T { a: 2, ..s0 };
38+
//~^ ERROR [E0509]
39+
}
40+
41+
fn b(s0: T) {
42+
let _s2 = T { ..s0 };
43+
//~^ ERROR [E0509]
44+
}
45+
46+
fn c(s0: T) {
47+
let _s2 = T { a: 2, b: s0.b };
48+
//~^ ERROR [E0509]
49+
}
50+
51+
fn d<K: Clone>(s0: V<K>) {
52+
let _s2 = V { a: 2, ..s0 };
53+
//~^ ERROR [E0509]
54+
//~| ERROR [E0509]
55+
}
1556

16-
fn g(s0:T) {
17-
let _s2 = T{a: 2, ..s0};
18-
//~^ ERROR [E0509]
57+
fn e(s0: V<Clonable>) {
58+
let _s2 = V { a: 2, ..s0 };
59+
//~^ ERROR [E0509]
60+
//~| ERROR [E0509]
61+
}
1962
}
2063

2164
fn main() { }

0 commit comments

Comments
 (0)