Skip to content

Commit d578ac9

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 4ca876b commit d578ac9

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
@@ -999,13 +999,109 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
999999
can_suggest_clone
10001000
}
10011001

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

1203+
fn implements_clone(&self, ty: Ty<'tcx>) -> bool {
1204+
let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait() else { return false };
1205+
self.infcx
1206+
.type_implements_trait(clone_trait_def, [ty], self.param_env)
1207+
.must_apply_modulo_regions()
1208+
}
1209+
11141210
pub(crate) fn clone_on_reference(&self, expr: &hir::Expr<'_>) -> Option<Span> {
11151211
let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
11161212
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)