Skip to content

Commit 8dc36c1

Browse files
linyihailinyihai
committed
fix: prefer (*p).clone to p.clone if the p is a raw pointer
1 parent d38cd22 commit 8dc36c1

5 files changed

Lines changed: 50 additions & 33 deletions

File tree

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,14 +1288,16 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
12881288
return false;
12891289
}
12901290
// Try to find predicates on *generic params* that would allow copying `ty`
1291-
let suggestion =
1291+
let mut suggestion =
12921292
if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
12931293
format!(": {symbol}.clone()")
12941294
} else {
12951295
".clone()".to_owned()
12961296
};
12971297
let mut sugg = Vec::with_capacity(2);
12981298
let mut inner_expr = expr;
1299+
let mut is_raw_ptr = false;
1300+
let typeck_result = self.infcx.tcx.typeck(self.mir_def_id());
12991301
// Remove uses of `&` and `*` when suggesting `.clone()`.
13001302
while let hir::ExprKind::AddrOf(.., inner) | hir::ExprKind::Unary(hir::UnOp::Deref, inner) =
13011303
&inner_expr.kind
@@ -1306,14 +1308,32 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
13061308
return false;
13071309
}
13081310
inner_expr = inner;
1311+
if let Some(inner_type) = typeck_result.node_type_opt(inner.hir_id) {
1312+
if matches!(inner_type.kind(), ty::RawPtr(..)) {
1313+
is_raw_ptr = true;
1314+
break;
1315+
}
1316+
}
13091317
}
1310-
if inner_expr.span.lo() != expr.span.lo() {
1318+
// Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch error. (see #126863)
1319+
if inner_expr.span.lo() != expr.span.lo() && !is_raw_ptr {
1320+
// Remove "(*" or "(&"
13111321
sugg.push((expr.span.with_hi(inner_expr.span.lo()), String::new()));
13121322
}
1323+
// Check whether `expr` is surrounded by parentheses or not.
13131324
let span = if inner_expr.span.hi() != expr.span.hi() {
13141325
// Account for `(*x)` to suggest `x.clone()`.
1315-
expr.span.with_lo(inner_expr.span.hi())
1326+
if is_raw_ptr {
1327+
expr.span.shrink_to_hi()
1328+
} else {
1329+
// Remove the close parenthesis ")"
1330+
expr.span.with_lo(inner_expr.span.hi())
1331+
}
13161332
} else {
1333+
if is_raw_ptr {
1334+
sugg.push((expr.span.shrink_to_lo(), "(".to_string()));
1335+
suggestion = ").clone()".to_string();
1336+
}
13171337
expr.span.shrink_to_hi()
13181338
};
13191339
sugg.push((span, suggestion));

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -639,12 +639,27 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
639639
fn add_borrow_suggestions(&self, err: &mut Diag<'_>, span: Span) {
640640
match self.infcx.tcx.sess.source_map().span_to_snippet(span) {
641641
Ok(snippet) if snippet.starts_with('*') => {
642-
err.span_suggestion_verbose(
643-
span.with_hi(span.lo() + BytePos(1)),
644-
"consider removing the dereference here",
645-
String::new(),
646-
Applicability::MaybeIncorrect,
647-
);
642+
let sp = span.with_lo(span.lo() + BytePos(1));
643+
let inner = self.find_expr(sp);
644+
let mut is_raw_ptr = false;
645+
if let Some(inner) = inner {
646+
let typck_result = self.infcx.tcx.typeck(self.mir_def_id());
647+
if let Some(inner_type) = typck_result.node_type_opt(inner.hir_id) {
648+
if matches!(inner_type.kind(), ty::RawPtr(..)) {
649+
is_raw_ptr = true;
650+
}
651+
}
652+
}
653+
// If the `inner` is a raw pointer, do not suggest removing the "*", see #126863
654+
// FIXME: need to check whether the assigned object can be a raw pointer, see `tests/ui/borrowck/issue-20801.rs`.
655+
if !is_raw_ptr {
656+
err.span_suggestion_verbose(
657+
span.with_hi(span.lo() + BytePos(1)),
658+
"consider removing the dereference here",
659+
String::new(),
660+
Applicability::MaybeIncorrect,
661+
);
662+
}
648663
}
649664
_ => {
650665
err.span_suggestion_verbose(

‎tests/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,10 @@ error[E0507]: cannot move out of `*x` which is behind a raw pointer
44
LL | let y = *x;
55
| ^^ move occurs because `*x` has type `Box<isize>`, which does not implement the `Copy` trait
66
|
7-
help: consider removing the dereference here
8-
|
9-
LL - let y = *x;
10-
LL + let y = x;
11-
|
127
help: consider cloning the value if the performance cost is acceptable
138
|
14-
LL - let y = *x;
15-
LL + let y = x.clone();
16-
|
9+
LL | let y = (*x).clone();
10+
| + +++++++++
1711

1812
error: aborting due to 1 previous error
1913

‎tests/ui/borrowck/issue-20801.stderr‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,6 @@ LL | struct T(u8);
6767
...
6868
LL | let c = unsafe { *mut_ptr() };
6969
| ---------- you could clone this value
70-
help: consider removing the dereference here
71-
|
72-
LL - let c = unsafe { *mut_ptr() };
73-
LL + let c = unsafe { mut_ptr() };
74-
|
7570

7671
error[E0507]: cannot move out of a raw pointer
7772
--> $DIR/issue-20801.rs:36:22
@@ -87,11 +82,6 @@ LL | struct T(u8);
8782
...
8883
LL | let d = unsafe { *const_ptr() };
8984
| ------------ you could clone this value
90-
help: consider removing the dereference here
91-
|
92-
LL - let d = unsafe { *const_ptr() };
93-
LL + let d = unsafe { const_ptr() };
94-
|
9585

9686
error: aborting due to 4 previous errors; 1 warning emitted
9787

‎tests/ui/borrowck/move-from-union-field-issue-66500.stderr‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,8 @@ LL | *u.c
3030
|
3131
help: consider cloning the value if the performance cost is acceptable
3232
|
33-
LL - *u.c
34-
LL + u.c.clone()
35-
|
33+
LL | (*u.c).clone()
34+
| + +++++++++
3635

3736
error[E0507]: cannot move out of `*u.d` which is behind a raw pointer
3837
--> $DIR/move-from-union-field-issue-66500.rs:24:5
@@ -42,9 +41,8 @@ LL | *u.d
4241
|
4342
help: consider cloning the value if the performance cost is acceptable
4443
|
45-
LL - *u.d
46-
LL + u.d.clone()
47-
|
44+
LL | (*u.d).clone()
45+
| + +++++++++
4846

4947
error: aborting due to 4 previous errors
5048

0 commit comments

Comments
 (0)