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

Author: linyihai

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -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
@@ -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));

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -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(

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

@@ -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
 

tests/ui/borrowck/issue-20801.stderr

@@ -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
@@ -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
 

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

@@ -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
@@ -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