Skip to content

Commit

Permalink
Unrolled build for rust-lang#127114
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#127114 - linyihai:issue-126863, r=Nadrieril

fix: prefer `(*p).clone` to `p.clone` if the `p` is a raw pointer

Fixes rust-lang#126863

I wonder if there is a better way to solve the regression problem of this test case:
`tests/ui/borrowck/issue-20801.rs`.
It's okay to drop the dereference symbol in this scenario.

But it's not correct in rust-lang#126863

```
help: consider removing the dereference here
  |
5 -         let inner: String = *p;
5 +         let inner: String = p;
```

I haven't found out how to tell if clone pointer is allowed, i.e. no type mismatch occurs
  • Loading branch information
rust-timer authored Jun 30, 2024
2 parents bf750f5 + 8dc36c1 commit ac9bde4
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 33 deletions.
26 changes: 23 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1288,14 +1288,16 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
return false;
}
// Try to find predicates on *generic params* that would allow copying `ty`
let suggestion =
let mut suggestion =
if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {
format!(": {symbol}.clone()")
} else {
".clone()".to_owned()
};
let mut sugg = Vec::with_capacity(2);
let mut inner_expr = expr;
let mut is_raw_ptr = false;
let typeck_result = self.infcx.tcx.typeck(self.mir_def_id());
// Remove uses of `&` and `*` when suggesting `.clone()`.
while let hir::ExprKind::AddrOf(.., inner) | hir::ExprKind::Unary(hir::UnOp::Deref, inner) =
&inner_expr.kind
Expand All @@ -1306,14 +1308,32 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
return false;
}
inner_expr = inner;
if let Some(inner_type) = typeck_result.node_type_opt(inner.hir_id) {
if matches!(inner_type.kind(), ty::RawPtr(..)) {
is_raw_ptr = true;
break;
}
}
}
if inner_expr.span.lo() != expr.span.lo() {
// Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch error. (see #126863)
if inner_expr.span.lo() != expr.span.lo() && !is_raw_ptr {
// Remove "(*" or "(&"
sugg.push((expr.span.with_hi(inner_expr.span.lo()), String::new()));
}
// Check whether `expr` is surrounded by parentheses or not.
let span = if inner_expr.span.hi() != expr.span.hi() {
// Account for `(*x)` to suggest `x.clone()`.
expr.span.with_lo(inner_expr.span.hi())
if is_raw_ptr {
expr.span.shrink_to_hi()
} else {
// Remove the close parenthesis ")"
expr.span.with_lo(inner_expr.span.hi())
}
} else {
if is_raw_ptr {
sugg.push((expr.span.shrink_to_lo(), "(".to_string()));
suggestion = ").clone()".to_string();
}
expr.span.shrink_to_hi()
};
sugg.push((span, suggestion));
Expand Down
27 changes: 21 additions & 6 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,12 +639,27 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
fn add_borrow_suggestions(&self, err: &mut Diag<'_>, span: Span) {
match self.infcx.tcx.sess.source_map().span_to_snippet(span) {
Ok(snippet) if snippet.starts_with('*') => {
err.span_suggestion_verbose(
span.with_hi(span.lo() + BytePos(1)),
"consider removing the dereference here",
String::new(),
Applicability::MaybeIncorrect,
);
let sp = span.with_lo(span.lo() + BytePos(1));
let inner = self.find_expr(sp);
let mut is_raw_ptr = false;
if let Some(inner) = inner {
let typck_result = self.infcx.tcx.typeck(self.mir_def_id());
if let Some(inner_type) = typck_result.node_type_opt(inner.hir_id) {
if matches!(inner_type.kind(), ty::RawPtr(..)) {
is_raw_ptr = true;
}
}
}
// If the `inner` is a raw pointer, do not suggest removing the "*", see #126863
// FIXME: need to check whether the assigned object can be a raw pointer, see `tests/ui/borrowck/issue-20801.rs`.
if !is_raw_ptr {
err.span_suggestion_verbose(
span.with_hi(span.lo() + BytePos(1)),
"consider removing the dereference here",
String::new(),
Applicability::MaybeIncorrect,
);
}
}
_ => {
err.span_suggestion_verbose(
Expand Down
10 changes: 2 additions & 8 deletions tests/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,10 @@ error[E0507]: cannot move out of `*x` which is behind a raw pointer
LL | let y = *x;
| ^^ move occurs because `*x` has type `Box<isize>`, which does not implement the `Copy` trait
|
help: consider removing the dereference here
|
LL - let y = *x;
LL + let y = x;
|
help: consider cloning the value if the performance cost is acceptable
|
LL - let y = *x;
LL + let y = x.clone();
|
LL | let y = (*x).clone();
| + +++++++++

error: aborting due to 1 previous error

Expand Down
10 changes: 0 additions & 10 deletions tests/ui/borrowck/issue-20801.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ LL | struct T(u8);
...
LL | let c = unsafe { *mut_ptr() };
| ---------- you could clone this value
help: consider removing the dereference here
|
LL - let c = unsafe { *mut_ptr() };
LL + let c = unsafe { mut_ptr() };
|

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

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

Expand Down
10 changes: 4 additions & 6 deletions tests/ui/borrowck/move-from-union-field-issue-66500.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ LL | *u.c
|
help: consider cloning the value if the performance cost is acceptable
|
LL - *u.c
LL + u.c.clone()
|
LL | (*u.c).clone()
| + +++++++++

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

error: aborting due to 4 previous errors

Expand Down

0 comments on commit ac9bde4

Please sign in to comment.