Provide more suggestions for cloning immutable bindings

estebank · estebank · commit cc80106cb5dd · 2023-12-04T21:54:34.000Z
When encountering multiple mutable borrows, suggest cloning and adding
derive annotations as needed.

```
error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&amp;` reference
  --&gt; $DIR/accidentally-cloning-ref-borrow-error.rs:32:9
   |
LL |     foo(&amp;mut sm.x);
   |         ^^^^^^^^^ `sm` is a `&amp;` reference, so the data it refers to cannot be borrowed as mutable
   |
help: `Str` doesn't implement `Clone`, so this call clones the reference `&amp;Str`
  --&gt; $DIR/accidentally-cloning-ref-borrow-error.rs:31:21
   |
LL |     let mut sm = sr.clone();
   |                     ^^^^^^^
help: consider annotating `Str` with `#[derive(Clone)]`
   |
LL + #[derive(Clone)]
LL | struct Str {
   |
help: consider specifying this binding's type
   |
LL |     let mut sm: &amp;mut Str = sr.clone();
   |               ++++++++++
```

```
error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&amp;` reference
  --&gt; $DIR/issue-91206.rs:14:5
   |
LL |     inner.clear();
   |     ^^^^^ `inner` is a `&amp;` reference, so the data it refers to cannot be borrowed as mutable
   |
help: you can `clone` the `Vec&lt;usize&gt;` value and consume it, but this might not be your desired behavior
  --&gt; $DIR/issue-91206.rs:11:17
   |
LL |     let inner = client.get_inner_ref();
   |                 ^^^^^^^^^^^^^^^^^^^^^^
help: consider specifying this binding's type
   |
LL |     let inner: &amp;mut Vec&lt;usize&gt; = client.get_inner_ref();
   |              +++++++++++++++++
```
diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -3,15 +3,18 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
 use rustc_hir as hir;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::Node;
+use rustc_infer::traits;
 use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
-use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt};
+use rustc_middle::ty::{self, InstanceDef, ToPredicate, Ty, TyCtxt};
 use rustc_middle::{
     hir::place::PlaceBase,
     mir::{self, BindingForm, Local, LocalDecl, LocalInfo, LocalKind, Location},
 };
 use rustc_span::symbol::{kw, Symbol};
 use rustc_span::{sym, BytePos, DesugaringKind, Span};
 use rustc_target::abi::FieldIdx;
+use rustc_trait_selection::infer::InferCtxtExt;
+use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt;
 
 use crate::diagnostics::BorrowedContentSource;
 use crate::util::FindAssignments;
@@ -1212,6 +1215,103 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 if let Some(hir_id) = hir_id
                     && let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
                 {
+                    let tables = self.infcx.tcx.typeck(def_id.as_local().unwrap());
+                    if let Some(clone_trait) = self.infcx.tcx.lang_items().clone_trait()
+                        && let Some(expr) = local.init
+                        && let ty = tables.node_type_opt(expr.hir_id)
+                        && let Some(ty) = ty
+                        && let ty::Ref(..) = ty.kind()
+                    {
+                        match self
+                            .infcx
+                            .could_impl_trait(clone_trait, ty.peel_refs(), self.param_env)
+                            .as_deref()
+                        {
+                            Some([]) => {
+                                // The type implements Clone.
+                                err.span_help(
+                                    expr.span,
+                                    format!(
+                                        "you can `clone` the `{}` value and consume it, but this \
+                                         might not be your desired behavior",
+                                        ty.peel_refs(),
+                                    ),
+                                );
+                            }
+                            None => {
+                                if let hir::ExprKind::MethodCall(segment, _rcvr, [], span) =
+                                    expr.kind
+                                    && segment.ident.name == sym::clone
+                                {
+                                    err.span_help(
+                                        span,
+                                        format!(
+                                            "`{}` doesn't implement `Clone`, so this call clones \
+                                             the reference `{ty}`",
+                                            ty.peel_refs(),
+                                        ),
+                                    );
+                                }
+                                // The type doesn't implement Clone.
+                                let trait_ref = ty::Binder::dummy(ty::TraitRef::new(
+                                    self.infcx.tcx,
+                                    clone_trait,
+                                    [ty.peel_refs()],
+                                ));
+                                let obligation = traits::Obligation::new(
+                                    self.infcx.tcx,
+                                    traits::ObligationCause::dummy(),
+                                    self.param_env,
+                                    trait_ref,
+                                );
+                                self.infcx.err_ctxt().suggest_derive(
+                                    &obligation,
+                                    err,
+                                    trait_ref.to_predicate(self.infcx.tcx),
+                                );
+                            }
+                            Some(errors) => {
+                                if let hir::ExprKind::MethodCall(segment, _rcvr, [], span) =
+                                    expr.kind
+                                    && segment.ident.name == sym::clone
+                                {
+                                    err.span_help(
+                                        span,
+                                        format!(
+                                            "`{}` doesn't implement `Clone` because its \
+                                             implementations trait bounds could not be met, so \
+                                             this call clones the reference `{ty}`",
+                                            ty.peel_refs(),
+                                        ),
+                                    );
+                                    err.note(format!(
+                                        "the following trait bounds weren't met: {}",
+                                        errors
+                                            .iter()
+                                            .map(|e| e.obligation.predicate.to_string())
+                                            .collect::<Vec<_>>()
+                                            .join("\n"),
+                                    ));
+                                }
+                                // The type doesn't implement Clone because of unmet obligations.
+                                for error in errors {
+                                    if let traits::FulfillmentErrorCode::CodeSelectionError(
+                                        traits::SelectionError::Unimplemented,
+                                    ) = error.code
+                                        && let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
+                                            pred,
+                                        )) = error.obligation.predicate.kind().skip_binder()
+                                    {
+                                        self.infcx.err_ctxt().suggest_derive(
+                                            &error.obligation,
+                                            err,
+                                            error.obligation.predicate.kind().rebind(pred),
+                                        );
+                                    }
+                                }
+                            }
+                        }
+                    }
                     let (changing, span, sugg) = match local.ty {
                         Some(ty) => ("changing", ty.span, message),
                         None => {
diff --git a/tests/ui/borrowck/accidentally-cloning-ref-borrow-error.rs b/tests/ui/borrowck/accidentally-cloning-ref-borrow-error.rs
@@ -0,0 +1,38 @@
+#[derive(Debug)]
+struct X<T>(T);
+
+impl<T: Clone> Clone for X<T> {
+    fn clone(&self) -> X<T> {
+        X(self.0.clone())
+    }
+}
+
+#[derive(Debug)]
+struct Y;
+
+#[derive(Debug)]
+struct Str {
+   x: Option<i32>,
+}
+
+fn foo(s: &mut Option<i32>) {
+    if s.is_none() {
+        *s = Some(0);
+    }
+    println!("{:?}", s);
+}
+
+fn bar<T: std::fmt::Debug>(s: &mut X<T>) {
+    println!("{:?}", s);
+}
+fn main() {
+    let s = Str { x: None };
+    let sr = &s;
+    let mut sm = sr.clone();
+    foo(&mut sm.x); //~ ERROR cannot borrow `sm.x` as mutable, as it is behind a `&` reference
+
+    let x = X(Y);
+    let xr = &x;
+    let mut xm = xr.clone();
+    bar(&mut xm); //~ ERROR cannot borrow data in a `&` reference as mutable
+}
diff --git a/tests/ui/borrowck/accidentally-cloning-ref-borrow-error.stderr b/tests/ui/borrowck/accidentally-cloning-ref-borrow-error.stderr
@@ -0,0 +1,30 @@
+error[E0596]: cannot borrow `sm.x` as mutable, as it is behind a `&` reference
+  --> $DIR/accidentally-cloning-ref-borrow-error.rs:32:9
+   |
+LL |     foo(&mut sm.x);
+   |         ^^^^^^^^^ `sm` is a `&` reference, so the data it refers to cannot be borrowed as mutable
+   |
+help: `Str` doesn't implement `Clone`, so this call clones the reference `&Str`
+  --> $DIR/accidentally-cloning-ref-borrow-error.rs:31:21
+   |
+LL |     let mut sm = sr.clone();
+   |                     ^^^^^^^
+help: consider annotating `Str` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct Str {
+   |
+help: consider specifying this binding's type
+   |
+LL |     let mut sm: &mut Str = sr.clone();
+   |               ++++++++++
+
+error[E0596]: cannot borrow data in a `&` reference as mutable
+  --> $DIR/accidentally-cloning-ref-borrow-error.rs:37:9
+   |
+LL |     bar(&mut xm);
+   |         ^^^^^^^ cannot borrow as mutable
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0596`.
diff --git a/tests/ui/borrowck/issue-85765-closure.rs b/tests/ui/borrowck/issue-85765-closure.rs
@@ -3,6 +3,7 @@ fn main() {
         let mut test = Vec::new();
         let rofl: &Vec<Vec<i32>> = &mut test;
         //~^ HELP consider changing this binding's type
+        //~| HELP you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
         rofl.push(Vec::new());
         //~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
         //~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
diff --git a/tests/ui/borrowck/issue-85765-closure.stderr b/tests/ui/borrowck/issue-85765-closure.stderr
@@ -1,16 +1,21 @@
 error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
-  --> $DIR/issue-85765-closure.rs:6:9
+  --> $DIR/issue-85765-closure.rs:7:9
    |
 LL |         rofl.push(Vec::new());
    |         ^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
    |
+help: you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
+  --> $DIR/issue-85765-closure.rs:4:36
+   |
+LL |         let rofl: &Vec<Vec<i32>> = &mut test;
+   |                                    ^^^^^^^^^
 help: consider changing this binding's type
    |
 LL |         let rofl: &mut Vec<Vec<i32>> = &mut test;
    |                   ~~~~~~~~~~~~~~~~~~
 
 error[E0594]: cannot assign to `*r`, which is behind a `&` reference
-  --> $DIR/issue-85765-closure.rs:13:9
+  --> $DIR/issue-85765-closure.rs:14:9
    |
 LL |         *r = 0;
    |         ^^^^^^ `r` is a `&` reference, so the data it refers to cannot be written
@@ -21,7 +26,7 @@ LL |         let r = &mut mutvar;
    |                  +++
 
 error[E0594]: cannot assign to `*x`, which is behind a `&` reference
-  --> $DIR/issue-85765-closure.rs:20:9
+  --> $DIR/issue-85765-closure.rs:21:9
    |
 LL |         *x = 1;
    |         ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@@ -32,7 +37,7 @@ LL |         let x: &mut usize = &mut{0};
    |                ~~~~~~~~~~
 
 error[E0594]: cannot assign to `*y`, which is behind a `&` reference
-  --> $DIR/issue-85765-closure.rs:27:9
+  --> $DIR/issue-85765-closure.rs:28:9
    |
 LL |         *y = 1;
    |         ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written
diff --git a/tests/ui/borrowck/issue-85765.rs b/tests/ui/borrowck/issue-85765.rs
@@ -2,6 +2,7 @@ fn main() {
     let mut test = Vec::new();
     let rofl: &Vec<Vec<i32>> = &mut test;
     //~^ HELP consider changing this binding's type
+    //~| HELP you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
     rofl.push(Vec::new());
     //~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
     //~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
diff --git a/tests/ui/borrowck/issue-85765.stderr b/tests/ui/borrowck/issue-85765.stderr
@@ -1,16 +1,21 @@
 error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
-  --> $DIR/issue-85765.rs:5:5
+  --> $DIR/issue-85765.rs:6:5
    |
 LL |     rofl.push(Vec::new());
    |     ^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
    |
+help: you can `clone` the `Vec<Vec<i32>>` value and consume it, but this might not be your desired behavior
+  --> $DIR/issue-85765.rs:3:32
+   |
+LL |     let rofl: &Vec<Vec<i32>> = &mut test;
+   |                                ^^^^^^^^^
 help: consider changing this binding's type
    |
 LL |     let rofl: &mut Vec<Vec<i32>> = &mut test;
    |               ~~~~~~~~~~~~~~~~~~
 
 error[E0594]: cannot assign to `*r`, which is behind a `&` reference
-  --> $DIR/issue-85765.rs:12:5
+  --> $DIR/issue-85765.rs:13:5
    |
 LL |     *r = 0;
    |     ^^^^^^ `r` is a `&` reference, so the data it refers to cannot be written
@@ -21,7 +26,7 @@ LL |     let r = &mut mutvar;
    |              +++
 
 error[E0594]: cannot assign to `*x`, which is behind a `&` reference
-  --> $DIR/issue-85765.rs:19:5
+  --> $DIR/issue-85765.rs:20:5
    |
 LL |     *x = 1;
    |     ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@@ -32,7 +37,7 @@ LL |     let x: &mut usize = &mut{0};
    |            ~~~~~~~~~~
 
 error[E0594]: cannot assign to `*y`, which is behind a `&` reference
-  --> $DIR/issue-85765.rs:26:5
+  --> $DIR/issue-85765.rs:27:5
    |
 LL |     *y = 1;
    |     ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written
diff --git a/tests/ui/borrowck/issue-91206.rs b/tests/ui/borrowck/issue-91206.rs
@@ -10,6 +10,7 @@ fn main() {
     let client = TestClient;
     let inner = client.get_inner_ref();
     //~^ HELP consider specifying this binding's type
+    //~| HELP you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior
     inner.clear();
     //~^ ERROR cannot borrow `*inner` as mutable, as it is behind a `&` reference [E0596]
     //~| NOTE `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
diff --git a/tests/ui/borrowck/issue-91206.stderr b/tests/ui/borrowck/issue-91206.stderr
@@ -1,9 +1,14 @@
 error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference
-  --> $DIR/issue-91206.rs:13:5
+  --> $DIR/issue-91206.rs:14:5
    |
 LL |     inner.clear();
    |     ^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
    |
+help: you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior
+  --> $DIR/issue-91206.rs:11:17
+   |
+LL |     let inner = client.get_inner_ref();
+   |                 ^^^^^^^^^^^^^^^^^^^^^^
 help: consider specifying this binding's type
    |
 LL |     let inner: &mut Vec<usize> = client.get_inner_ref();

Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,14 @@`
`1`	`1`	error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference
`2`		`- --> $DIR/issue-91206.rs:13:5`
	`2`	`+ --> $DIR/issue-91206.rs:14:5`
`3`	`3`	`\|`
`4`	`4`	`LL \| inner.clear();`
`5`	`5`	\| ^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
`6`	`6`	`\|`
	`7`	+help: you can `clone` the `Vec<usize>` value and consume it, but this might not be your desired behavior
	`8`	`+ --> $DIR/issue-91206.rs:11:17`
	`9`	`+ \|`
	`10`	`+LL \| let inner = client.get_inner_ref();`
	`11`	`+ \| ^^^^^^^^^^^^^^^^^^^^^^`
`7`	`12`	`help: consider specifying this binding's type`
`8`	`13`	`\|`
`9`	`14`	`LL \| let inner: &mut Vec<usize> = client.get_inner_ref();`