Auto merge of rust-lang#106095 - estebank:pin-mut-reborrow, r=compiler-errors

bors · bors · commit 0ca50032ce72 · 2022-12-27T07:31:42.000Z
Suggest `Pin::as_mut` when encountering borrow error Fix rust-lang#65409 for `Pin<&mut T>`.
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -194,7 +194,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
 
                 if !seen_spans.contains(&move_span) {
                     if !closure {
-                        self.suggest_ref_or_clone(mpi, move_span, &mut err, &mut in_pattern);
+                        self.suggest_ref_or_clone(
+                            mpi,
+                            move_span,
+                            &mut err,
+                            &mut in_pattern,
+                            move_spans,
+                        );
                     }
 
                     self.explain_captures(
@@ -312,6 +318,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         move_span: Span,
         err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
         in_pattern: &mut bool,
+        move_spans: UseSpans<'_>,
     ) {
         struct ExpressionFinder<'hir> {
             expr_span: Span,
@@ -440,6 +447,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         ) = call_expr.kind
                     {
                         // Do not suggest `.clone()` in a `for` loop, we already suggest borrowing.
+                    } else if let UseSpans::FnSelfUse {
+                        kind: CallKind::Normal { .. },
+                        ..
+                    } = move_spans {
+                        // We already suggest cloning for these cases in `explain_captures`.
                     } else {
                         self.suggest_cloning(err, ty, move_span);
                     }
diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs
@@ -6,7 +6,7 @@ use rustc_errors::{Applicability, Diagnostic};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, Namespace};
 use rustc_hir::GeneratorKind;
-use rustc_infer::infer::TyCtxtInferExt;
+use rustc_infer::infer::{LateBoundRegionConversionTime, TyCtxtInferExt};
 use rustc_middle::mir::tcx::PlaceTy;
 use rustc_middle::mir::{
     AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand,
@@ -18,7 +18,10 @@ use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
 use rustc_span::def_id::LocalDefId;
 use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
 use rustc_target::abi::VariantIdx;
-use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;
+use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
+use rustc_trait_selection::traits::{
+    type_known_to_meet_bound_modulo_regions, Obligation, ObligationCause,
+};
 
 use super::borrow_set::BorrowData;
 use super::MirBorrowckCtxt;
@@ -1066,18 +1069,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 }
                 CallKind::Normal { self_arg, desugaring, method_did } => {
                     let self_arg = self_arg.unwrap();
+                    let tcx = self.infcx.tcx;
                     if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
-                        let ty = moved_place.ty(self.body, self.infcx.tcx).ty;
-                        let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) {
+                        let ty = moved_place.ty(self.body, tcx).ty;
+                        let suggest = match tcx.get_diagnostic_item(sym::IntoIterator) {
                             Some(def_id) => {
                                 let infcx = self.infcx.tcx.infer_ctxt().build();
                                 type_known_to_meet_bound_modulo_regions(
                                     &infcx,
                                     self.param_env,
-                                    infcx.tcx.mk_imm_ref(
-                                        infcx.tcx.lifetimes.re_erased,
-                                        infcx.tcx.erase_regions(ty),
-                                    ),
+                                    tcx.mk_imm_ref(tcx.lifetimes.re_erased, tcx.erase_regions(ty)),
                                     def_id,
                                     DUMMY_SP,
                                 )
@@ -1133,8 +1134,44 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 place_name, partially_str, loop_message
                             ),
                         );
+                        let infcx = tcx.infer_ctxt().build();
+                        let ty = tcx.erase_regions(moved_place.ty(self.body, tcx).ty);
+                        if let ty::Adt(def, substs) = ty.kind()
+                            && Some(def.did()) == tcx.lang_items().pin_type()
+                            && let ty::Ref(_, _, hir::Mutability::Mut) = substs.type_at(0).kind()
+                            && let self_ty = infcx.replace_bound_vars_with_fresh_vars(
+                                fn_call_span,
+                                LateBoundRegionConversionTime::FnCall,
+                                tcx.fn_sig(method_did).input(0),
+                            )
+                            && infcx.can_eq(self.param_env, ty, self_ty).is_ok()
+                        {
+                            err.span_suggestion_verbose(
+                                fn_call_span.shrink_to_lo(),
+                                "consider reborrowing the `Pin` instead of moving it",
+                                "as_mut().".to_string(),
+                                Applicability::MaybeIncorrect,
+                            );
+                        }
+                        if let Some(clone_trait) = tcx.lang_items().clone_trait()
+                            && let trait_ref = tcx.mk_trait_ref(clone_trait, [ty])
+                            && let o = Obligation::new(
+                                tcx,
+                                ObligationCause::dummy(),
+                                self.param_env,
+                                ty::Binder::dummy(trait_ref),
+                            )
+                            && infcx.predicate_must_hold_modulo_regions(&o)
+                        {
+                            err.span_suggestion_verbose(
+                                fn_call_span.shrink_to_lo(),
+                                "you can `clone` the value and consume it, but this might not be \
+                                 your desired behavior",
+                                "clone().".to_string(),
+                                Applicability::MaybeIncorrect,
+                            );
+                        }
                     }
-                    let tcx = self.infcx.tcx;
                     // Avoid pointing to the same function in multiple different
                     // error messages.
                     if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
diff --git a/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr b/src/test/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr
@@ -9,6 +9,10 @@ LL |     let _x = Rc::new(vec![1, 2]).into_iter();
    |
 note: `into_iter` takes ownership of the receiver `self`, which moves value
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |     let _x = Rc::new(vec![1, 2]).clone().into_iter();
+   |                                  ++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed
@@ -0,0 +1,15 @@
+// run-rustfix
+// Test that a by-ref `FnMut` closure gets an error when it tries to
+// consume a value.
+
+fn call<F>(f: F) where F : Fn() {
+    f();
+}
+
+fn main() {
+    let y = vec![format!("World")];
+    call(|| {
+        y.clone().into_iter();
+        //~^ ERROR cannot move out of `y`, a captured variable in an `Fn` closure
+    });
+}
diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.rs b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.rs
@@ -1,3 +1,4 @@
+// run-rustfix
 // Test that a by-ref `FnMut` closure gets an error when it tries to
 // consume a value.
 
diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr
@@ -1,5 +1,5 @@
 error[E0507]: cannot move out of `y`, a captured variable in an `Fn` closure
-  --> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:11:9
+  --> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:12:9
    |
 LL |     let y = vec![format!("World")];
    |         - captured outer variable
@@ -12,6 +12,10 @@ LL |         y.into_iter();
    |
 note: `into_iter` takes ownership of the receiver `self`, which moves `y`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |         y.clone().into_iter();
+   |           ++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/codemap_tests/tab_3.stderr b/src/test/ui/codemap_tests/tab_3.stderr
@@ -12,10 +12,10 @@ LL |         println!("{:?}", some_vec);
 note: `into_iter` takes ownership of the receiver `self`, which moves `some_vec`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
    = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
-help: consider cloning the value if the performance cost is acceptable
+help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     some_vec.clone().into_iter();
-   |             ++++++++
+   |              ++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/moves/move-fn-self-receiver.stderr b/src/test/ui/moves/move-fn-self-receiver.stderr
@@ -9,6 +9,10 @@ LL |     val.0;
 note: `into_iter` takes ownership of the receiver `self`, which moves `val.0`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
    = note: move occurs because `val.0` has type `Vec<bool>`, which does not implement the `Copy` trait
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |     val.0.clone().into_iter().next();
+   |           ++++++++
 
 error[E0382]: use of moved value: `foo`
   --> $DIR/move-fn-self-receiver.rs:34:5
@@ -93,10 +97,10 @@ note: `Foo::use_rc_self` takes ownership of the receiver `self`, which moves `rc
    |
 LL |     fn use_rc_self(self: Rc<Self>) {}
    |                    ^^^^
-help: consider cloning the value if the performance cost is acceptable
+help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     rc_foo.clone().use_rc_self();
-   |           ++++++++
+   |            ++++++++
 
 error[E0382]: use of moved value: `foo_add`
   --> $DIR/move-fn-self-receiver.rs:59:5
@@ -136,10 +140,10 @@ LL |     for _val in explicit_into_iter.into_iter() {}
 LL |     explicit_into_iter;
    |     ^^^^^^^^^^^^^^^^^^ value used here after move
    |
-help: consider cloning the value if the performance cost is acceptable
+help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     for _val in explicit_into_iter.clone().into_iter() {}
-   |                                   ++++++++
+   |                                    ++++++++
 
 error[E0382]: use of moved value: `container`
   --> $DIR/move-fn-self-receiver.rs:71:5
diff --git a/src/test/ui/moves/moves-based-on-type-access-to-field.stderr b/src/test/ui/moves/moves-based-on-type-access-to-field.stderr
@@ -10,10 +10,10 @@ LL |     touch(&x[0]);
    |
 note: `into_iter` takes ownership of the receiver `self`, which moves `x`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
-help: consider cloning the value if the performance cost is acceptable
+help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     consume(x.clone().into_iter().next().unwrap());
-   |              ++++++++
+   |               ++++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/moves/moves-based-on-type-exprs.stderr b/src/test/ui/moves/moves-based-on-type-exprs.stderr
@@ -162,10 +162,10 @@ LL |     touch(&x);
    |
 note: `into_iter` takes ownership of the receiver `self`, which moves `x`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
-help: consider cloning the value if the performance cost is acceptable
+help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     let _y = x.clone().into_iter().next().unwrap();
-   |               ++++++++
+   |                ++++++++
 
 error[E0382]: borrow of moved value: `x`
   --> $DIR/moves-based-on-type-exprs.rs:83:11
@@ -179,10 +179,10 @@ LL |     touch(&x);
    |
 note: `into_iter` takes ownership of the receiver `self`, which moves `x`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
-help: consider cloning the value if the performance cost is acceptable
+help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
 LL |     let _y = [x.clone().into_iter().next().unwrap(); 1];
-   |                ++++++++
+   |                 ++++++++
 
 error: aborting due to 11 previous errors
 
diff --git a/src/test/ui/moves/pin-mut-reborrow.fixed b/src/test/ui/moves/pin-mut-reborrow.fixed
@@ -0,0 +1,15 @@
+// run-rustfix
+use std::pin::Pin;
+
+struct Foo;
+
+impl Foo {
+    fn foo(self: Pin<&mut Self>) {}
+}
+
+fn main() {
+    let mut foo = Foo;
+    let mut foo = Pin::new(&mut foo);
+    foo.as_mut().foo();
+    foo.foo(); //~ ERROR use of moved value
+}
diff --git a/src/test/ui/moves/pin-mut-reborrow.rs b/src/test/ui/moves/pin-mut-reborrow.rs
@@ -0,0 +1,15 @@
+// run-rustfix
+use std::pin::Pin;
+
+struct Foo;
+
+impl Foo {
+    fn foo(self: Pin<&mut Self>) {}
+}
+
+fn main() {
+    let mut foo = Foo;
+    let mut foo = Pin::new(&mut foo);
+    foo.foo();
+    foo.foo(); //~ ERROR use of moved value
+}
diff --git a/src/test/ui/moves/pin-mut-reborrow.stderr b/src/test/ui/moves/pin-mut-reborrow.stderr
@@ -0,0 +1,23 @@
+error[E0382]: use of moved value: `foo`
+  --> $DIR/pin-mut-reborrow.rs:14:5
+   |
+LL |     let mut foo = Pin::new(&mut foo);
+   |         ------- move occurs because `foo` has type `Pin<&mut Foo>`, which does not implement the `Copy` trait
+LL |     foo.foo();
+   |         ----- `foo` moved due to this method call
+LL |     foo.foo();
+   |     ^^^ value used here after move
+   |
+note: `Foo::foo` takes ownership of the receiver `self`, which moves `foo`
+  --> $DIR/pin-mut-reborrow.rs:7:12
+   |
+LL |     fn foo(self: Pin<&mut Self>) {}
+   |            ^^^^
+help: consider reborrowing the `Pin` instead of moving it
+   |
+LL |     foo.as_mut().foo();
+   |         +++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0382`.
diff --git a/src/test/ui/moves/suggest-clone.fixed b/src/test/ui/moves/suggest-clone.fixed
@@ -0,0 +1,11 @@
+// run-rustfix
+
+#[derive(Clone)]
+struct Foo;
+impl Foo {
+    fn foo(self) {}
+}
+fn main() {
+    let foo = &Foo;
+    foo.clone().foo(); //~ ERROR cannot move out
+}
diff --git a/src/test/ui/moves/suggest-clone.rs b/src/test/ui/moves/suggest-clone.rs
@@ -0,0 +1,11 @@
+// run-rustfix
+
+#[derive(Clone)]
+struct Foo;
+impl Foo {
+    fn foo(self) {}
+}
+fn main() {
+    let foo = &Foo;
+    foo.foo(); //~ ERROR cannot move out
+}
diff --git a/src/test/ui/moves/suggest-clone.stderr b/src/test/ui/moves/suggest-clone.stderr
@@ -0,0 +1,22 @@
+error[E0507]: cannot move out of `*foo` which is behind a shared reference
+  --> $DIR/suggest-clone.rs:10:5
+   |
+LL |     foo.foo();
+   |     ^^^^-----
+   |     |   |
+   |     |   `*foo` moved due to this method call
+   |     move occurs because `*foo` has type `Foo`, which does not implement the `Copy` trait
+   |
+note: `Foo::foo` takes ownership of the receiver `self`, which moves `*foo`
+  --> $DIR/suggest-clone.rs:6:12
+   |
+LL |     fn foo(self) {}
+   |            ^^^^
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |     foo.clone().foo();
+   |         ++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0507`.
diff --git a/src/test/ui/suggestions/option-content-move.stderr b/src/test/ui/suggestions/option-content-move.stderr
@@ -9,6 +9,10 @@ LL |                 if selection.1.unwrap().contains(selection.0) {
    |
 note: `Option::<T>::unwrap` takes ownership of the receiver `self`, which moves `selection.1`
   --> $SRC_DIR/core/src/option.rs:LL:COL
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |                 if selection.1.clone().unwrap().contains(selection.0) {
+   |                                ++++++++
 
 error[E0507]: cannot move out of `selection.1` which is behind a shared reference
   --> $DIR/option-content-move.rs:27:20
@@ -21,6 +25,10 @@ LL |                 if selection.1.unwrap().contains(selection.0) {
    |
 note: `Result::<T, E>::unwrap` takes ownership of the receiver `self`, which moves `selection.1`
   --> $SRC_DIR/core/src/result.rs:LL:COL
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |                 if selection.1.clone().unwrap().contains(selection.0) {
+   |                                ++++++++
 
 error: aborting due to 2 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,10 @@ LL \| let _x = Rc::new(vec![1, 2]).into_iter();`
`9`	`9`	`\|`
`10`	`10`	note: `into_iter` takes ownership of the receiver `self`, which moves value
`11`	`11`	`--> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL`
	`12`	+help: you can `clone` the value and consume it, but this might not be your desired behavior
	`13`	`+ \|`
	`14`	`+LL \| let _x = Rc::new(vec![1, 2]).clone().into_iter();`
	`15`	`+ \| ++++++++`
`12`	`16`
`13`	`17`	`error: aborting due to previous error`
`14`	`18`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+// run-rustfix`
`1`	`2`	// Test that a by-ref `FnMut` closure gets an error when it tries to
`2`	`3`	`// consume a value.`
`3`	`4`
Original file line number	Diff line number	Diff line change
`@@ -10,10 +10,10 @@ LL \| touch(&x[0]);`
`10`	`10`	`\|`
`11`	`11`	note: `into_iter` takes ownership of the receiver `self`, which moves `x`
`12`	`12`	`--> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL`
`13`		`-help: consider cloning the value if the performance cost is acceptable`
	`13`	+help: you can `clone` the value and consume it, but this might not be your desired behavior
`14`	`14`	`\|`
`15`	`15`	`LL \| consume(x.clone().into_iter().next().unwrap());`
`16`		`- \| ++++++++`
	`16`	`+ \| ++++++++`
`17`	`17`
`18`	`18`	`error: aborting due to previous error`
`19`	`19`
Original file line number	Diff line number	Diff line change
`@@ -162,10 +162,10 @@ LL \| touch(&x);`
`162`	`162`	`\|`
`163`	`163`	note: `into_iter` takes ownership of the receiver `self`, which moves `x`
`164`	`164`	`--> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL`
`165`		`-help: consider cloning the value if the performance cost is acceptable`
	`165`	+help: you can `clone` the value and consume it, but this might not be your desired behavior
`166`	`166`	`\|`
`167`	`167`	`LL \| let _y = x.clone().into_iter().next().unwrap();`
`168`		`- \| ++++++++`
	`168`	`+ \| ++++++++`
`169`	`169`
`170`	`170`	error[E0382]: borrow of moved value: `x`
`171`	`171`	`--> $DIR/moves-based-on-type-exprs.rs:83:11`
`@@ -179,10 +179,10 @@ LL \| touch(&x);`
`179`	`179`	`\|`
`180`	`180`	note: `into_iter` takes ownership of the receiver `self`, which moves `x`
`181`	`181`	`--> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL`
`182`		`-help: consider cloning the value if the performance cost is acceptable`
	`182`	+help: you can `clone` the value and consume it, but this might not be your desired behavior
`183`	`183`	`\|`
`184`	`184`	`LL \| let _y = [x.clone().into_iter().next().unwrap(); 1];`
`185`		`- \| ++++++++`
	`185`	`+ \| ++++++++`
`186`	`186`
`187`	`187`	`error: aborting due to 11 previous errors`
`188`	`188`