Rollup merge of #94460 - eholk:reenable-drop-tracking-tests, r=tmiasko

Reenable generator drop tracking tests and fix mutation handling

The previous PR, #94068, was overly zealous in counting mutations as borrows, which effectively nullified drop tracking. We would have caught this except the drop tracking tests were still ignored, despite having the option of using the `-Zdrop-tracking` flag now.

This PR fixes the issue introduced by #94068 by only counting mutations as borrows the mutated place has a project. This is sufficient to distinguish `x.y = 42` (which should count as a borrow of `x`) from `x = 42` (which is not a borrow of `x` because the whole variable is overwritten).

This PR also re-enables the drop tracking regression tests using the `-Zdrop-tracking` flag so we will avoid introducing these sorts of issues in the future.

Thanks to ``@tmiasko`` for noticing this problem and pointing it out!

r? ``@tmiasko``

10 files changed, 75 insertions, 52 deletions

diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs
index 03d3b23bb23..40ee6d863b5 100644
--- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs
+++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs
@@ -6,14 +6,14 @@ use crate::{
 use hir::{def_id::DefId, Body, HirId, HirIdMap};
 use rustc_data_structures::stable_set::FxHashSet;
 use rustc_hir as hir;
-use rustc_middle::hir::map::Map;
+use rustc_middle::ty::{ParamEnv, TyCtxt};
 
 pub(super) fn find_consumed_and_borrowed<'a, 'tcx>(
     fcx: &'a FnCtxt<'a, 'tcx>,
     def_id: DefId,
     body: &'tcx Body<'tcx>,
 ) -> ConsumedAndBorrowedPlaces {
-    let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx.hir());
+    let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx, fcx.param_env);
     expr_use_visitor.consume_body(fcx, def_id, body);
     expr_use_visitor.places
 }
@@ -36,14 +36,16 @@ pub(super) struct ConsumedAndBorrowedPlaces {
 /// Interesting values are those that are either dropped or borrowed. For dropped values, we also
 /// record the parent expression, which is the point where the drop actually takes place.
 struct ExprUseDelegate<'tcx> {
-    hir: Map<'tcx>,
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
     places: ConsumedAndBorrowedPlaces,
 }
 
 impl<'tcx> ExprUseDelegate<'tcx> {
-    fn new(hir: Map<'tcx>) -> Self {
+    fn new(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> Self {
         Self {
-            hir,
+            tcx,
+            param_env,
             places: ConsumedAndBorrowedPlaces {
                 consumed: <_>::default(),
                 borrowed: <_>::default(),
@@ -77,7 +79,7 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
         place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
         diag_expr_id: HirId,
     ) {
-        let parent = match self.hir.find_parent_node(place_with_id.hir_id) {
+        let parent = match self.tcx.hir().find_parent_node(place_with_id.hir_id) {
             Some(parent) => parent,
             None => place_with_id.hir_id,
         };
@@ -107,11 +109,22 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
         assignee_place: &expr_use_visitor::PlaceWithHirId<'tcx>,
         diag_expr_id: HirId,
     ) {
-        debug!("mutate {:?}; diag_expr_id={:?}", assignee_place, diag_expr_id);
-        // Count mutations as a borrow.
-        self.places
-            .borrowed
-            .insert(TrackedValue::from_place_with_projections_allowed(assignee_place));
+        debug!("mutate {assignee_place:?}; diag_expr_id={diag_expr_id:?}");
+        // If the type being assigned needs dropped, then the mutation counts as a borrow
+        // since it is essentially doing `Drop::drop(&mut x); x = new_value;`.
+        if assignee_place.place.base_ty.needs_drop(self.tcx, self.param_env) {
+            self.places
+                .borrowed
+                .insert(TrackedValue::from_place_with_projections_allowed(assignee_place));
+        }
+    }
+
+    fn bind(
+        &mut self,
+        binding_place: &expr_use_visitor::PlaceWithHirId<'tcx>,
+        diag_expr_id: HirId,
+    ) {
+        debug!("bind {binding_place:?}; diag_expr_id={diag_expr_id:?}");
     }
 
     fn fake_read(
diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs
index db1c80ae433..8c19bbd3214 100644
--- a/compiler/rustc_typeck/src/expr_use_visitor.rs
+++ b/compiler/rustc_typeck/src/expr_use_visitor.rs
@@ -51,6 +51,15 @@ pub trait Delegate<'tcx> {
     /// `diag_expr_id` is the id used for diagnostics (see `consume` for more details).
     fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId);
 
+    /// The path at `binding_place` is a binding that is being initialized.
+    ///
+    /// This covers cases such as `let x = 42;`
+    fn bind(&mut self, binding_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) {
+        // Bindings can normally be treated as a regular assignment, so by default we
+        // forward this to the mutate callback.
+        self.mutate(binding_place, diag_expr_id)
+    }
+
     /// The `place` should be a fake read because of specified `cause`.
     fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId);
 }
@@ -648,11 +657,9 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
                     let pat_ty = return_if_err!(mc.node_ty(pat.hir_id));
                     debug!("walk_pat: pat_ty={:?}", pat_ty);
 
-                    // Each match binding is effectively an assignment to the
-                    // binding being produced.
                     let def = Res::Local(canonical_id);
                     if let Ok(ref binding_place) = mc.cat_res(pat.hir_id, pat.span, pat_ty, def) {
-                        delegate.mutate(binding_place, binding_place.hir_id);
+                        delegate.bind(binding_place, binding_place.hir_id);
                     }
 
                     // It is also a borrow or copy/move of the value being matched.
diff --git a/src/test/ui/async-await/async-fn-nonsend.rs b/src/test/ui/async-await/async-fn-nonsend.rs
index a1a05c0acba..d7f8d7ac546 100644
--- a/src/test/ui/async-await/async-fn-nonsend.rs
+++ b/src/test/ui/async-await/async-fn-nonsend.rs
@@ -1,9 +1,5 @@
 // edition:2018
-// compile-flags: --crate-type lib
-
-// FIXME(eholk): temporarily disabled while drop range tracking is disabled
-// (see generator_interior.rs:27)
-// ignore-test
+// compile-flags: --crate-type lib -Zdrop-tracking
 
 use std::{cell::RefCell, fmt::Debug, rc::Rc};
 
diff --git a/src/test/ui/async-await/drop-and-assign.rs b/src/test/ui/async-await/drop-and-assign.rs
new file mode 100644
index 00000000000..fa3f3303677
--- /dev/null
+++ b/src/test/ui/async-await/drop-and-assign.rs
@@ -0,0 +1,19 @@
+// edition:2021
+// compile-flags: -Zdrop-tracking
+// build-pass
+
+struct A;
+impl Drop for A { fn drop(&mut self) {} }
+
+pub async fn f() {
+    let mut a = A;
+    a = A;
+    drop(a);
+    async {}.await;
+}
+
+fn assert_send<T: Send>(_: T) {}
+
+fn main() {
+    let _ = f();
+}
diff --git a/src/test/ui/async-await/unresolved_type_param.rs b/src/test/ui/async-await/unresolved_type_param.rs
index 187356ca140..6d6d8061491 100644
--- a/src/test/ui/async-await/unresolved_type_param.rs
+++ b/src/test/ui/async-await/unresolved_type_param.rs
@@ -2,10 +2,7 @@
 // Error message should pinpoint the type parameter T as needing to be bound
 // (rather than give a general error message)
 // edition:2018
-
-// FIXME(eholk): temporarily disabled while drop range tracking is disabled
-// (see generator_interior.rs:27)
-// ignore-test
+// compile-flags: -Zdrop-tracking
 
 async fn bar<T>() -> () {}
 
diff --git a/src/test/ui/async-await/unresolved_type_param.stderr b/src/test/ui/async-await/unresolved_type_param.stderr
index d19a3226ef9..7236c681f34 100644
--- a/src/test/ui/async-await/unresolved_type_param.stderr
+++ b/src/test/ui/async-await/unresolved_type_param.stderr
@@ -1,35 +1,35 @@
 error[E0698]: type inside `async fn` body must be known in this context
-  --> $DIR/unresolved_type_param.rs:9:5
+  --> $DIR/unresolved_type_param.rs:10:5
    |
 LL |     bar().await;
    |     ^^^ cannot infer type for type parameter `T` declared on the function `bar`
    |
 note: the type is part of the `async fn` body because of this `await`
-  --> $DIR/unresolved_type_param.rs:9:10
+  --> $DIR/unresolved_type_param.rs:10:10
    |
 LL |     bar().await;
    |          ^^^^^^
 
 error[E0698]: type inside `async fn` body must be known in this context
-  --> $DIR/unresolved_type_param.rs:9:5
+  --> $DIR/unresolved_type_param.rs:10:5
    |
 LL |     bar().await;
    |     ^^^ cannot infer type for type parameter `T` declared on the function `bar`
    |
 note: the type is part of the `async fn` body because of this `await`
-  --> $DIR/unresolved_type_param.rs:9:10
+  --> $DIR/unresolved_type_param.rs:10:10
    |
 LL |     bar().await;
    |          ^^^^^^
 
 error[E0698]: type inside `async fn` body must be known in this context
-  --> $DIR/unresolved_type_param.rs:9:5
+  --> $DIR/unresolved_type_param.rs:10:5
    |
 LL |     bar().await;
    |     ^^^ cannot infer type for type parameter `T` declared on the function `bar`
    |
 note: the type is part of the `async fn` body because of this `await`
-  --> $DIR/unresolved_type_param.rs:9:10
+  --> $DIR/unresolved_type_param.rs:10:10
    |
 LL |     bar().await;
    |          ^^^^^^
diff --git a/src/test/ui/generator/drop-control-flow.rs b/src/test/ui/generator/drop-control-flow.rs
index 914a7d71dc4..d383680002f 100644
--- a/src/test/ui/generator/drop-control-flow.rs
+++ b/src/test/ui/generator/drop-control-flow.rs
@@ -1,10 +1,6 @@
 // build-pass
 // compile-flags: -Zdrop-tracking
 
-// FIXME(eholk): temporarily disabled while drop range tracking is disabled
-// (see generator_interior.rs:27)
-// ignore-test
-
 // A test to ensure generators capture values that were conditionally dropped,
 // and also that values that are dropped along all paths to a yield do not get
 // included in the generator type.
diff --git a/src/test/ui/generator/issue-57478.rs b/src/test/ui/generator/issue-57478.rs
index 5c23ecbae32..91407ea1844 100644
--- a/src/test/ui/generator/issue-57478.rs
+++ b/src/test/ui/generator/issue-57478.rs
@@ -1,8 +1,5 @@
 // check-pass
-
-// FIXME(eholk): temporarily disabled while drop range tracking is disabled
-// (see generator_interior.rs:27)
-// ignore-test
+// compile-flags: -Zdrop-tracking
 
 #![feature(negative_impls, generators)]
 
diff --git a/src/test/ui/generator/partial-drop.rs b/src/test/ui/generator/partial-drop.rs
index e89e4b61bbf..c872fb7f3e6 100644
--- a/src/test/ui/generator/partial-drop.rs
+++ b/src/test/ui/generator/partial-drop.rs
@@ -1,6 +1,4 @@
-// FIXME(eholk): temporarily disabled while drop range tracking is disabled
-// (see generator_interior.rs:27)
-// ignore-test
+// compile-flags: -Zdrop-tracking
 
 #![feature(negative_impls, generators)]
 
diff --git a/src/test/ui/generator/partial-drop.stderr b/src/test/ui/generator/partial-drop.stderr
index 9a1b0734d8c..16b34c917ec 100644
--- a/src/test/ui/generator/partial-drop.stderr
+++ b/src/test/ui/generator/partial-drop.stderr
@@ -1,12 +1,12 @@
 error: generator cannot be sent between threads safely
-  --> $DIR/partial-drop.rs:12:5
+  --> $DIR/partial-drop.rs:14:5
    |
 LL |     assert_send(|| {
    |     ^^^^^^^^^^^ generator is not `Send`
    |
-   = help: within `[generator@$DIR/partial-drop.rs:12:17: 18:6]`, the trait `Send` is not implemented for `Foo`
+   = help: within `[generator@$DIR/partial-drop.rs:14:17: 20:6]`, the trait `Send` is not implemented for `Foo`
 note: generator is not `Send` as this value is used across a yield
-  --> $DIR/partial-drop.rs:17:9
+  --> $DIR/partial-drop.rs:19:9
    |
 LL |         let guard = Bar { foo: Foo, x: 42 };
    |             ----- has type `Bar` which is not `Send`
@@ -16,20 +16,20 @@ LL |         yield;
 LL |     });
    |     - `guard` is later dropped here
 note: required by a bound in `assert_send`
-  --> $DIR/partial-drop.rs:40:19
+  --> $DIR/partial-drop.rs:42:19
    |
 LL | fn assert_send<T: Send>(_: T) {}
    |                   ^^^^ required by this bound in `assert_send`
 
 error: generator cannot be sent between threads safely
-  --> $DIR/partial-drop.rs:20:5
+  --> $DIR/partial-drop.rs:22:5
    |
 LL |     assert_send(|| {
    |     ^^^^^^^^^^^ generator is not `Send`
    |
-   = help: within `[generator@$DIR/partial-drop.rs:20:17: 28:6]`, the trait `Send` is not implemented for `Foo`
+   = help: within `[generator@$DIR/partial-drop.rs:22:17: 30:6]`, the trait `Send` is not implemented for `Foo`
 note: generator is not `Send` as this value is used across a yield
-  --> $DIR/partial-drop.rs:27:9
+  --> $DIR/partial-drop.rs:29:9
    |
 LL |         let guard = Bar { foo: Foo, x: 42 };
    |             ----- has type `Bar` which is not `Send`
@@ -39,20 +39,20 @@ LL |         yield;
 LL |     });
    |     - `guard` is later dropped here
 note: required by a bound in `assert_send`
-  --> $DIR/partial-drop.rs:40:19
+  --> $DIR/partial-drop.rs:42:19
    |
 LL | fn assert_send<T: Send>(_: T) {}
    |                   ^^^^ required by this bound in `assert_send`
 
 error: generator cannot be sent between threads safely
-  --> $DIR/partial-drop.rs:30:5
+  --> $DIR/partial-drop.rs:32:5
    |
 LL |     assert_send(|| {
    |     ^^^^^^^^^^^ generator is not `Send`
    |
-   = help: within `[generator@$DIR/partial-drop.rs:30:17: 37:6]`, the trait `Send` is not implemented for `Foo`
+   = help: within `[generator@$DIR/partial-drop.rs:32:17: 39:6]`, the trait `Send` is not implemented for `Foo`
 note: generator is not `Send` as this value is used across a yield
-  --> $DIR/partial-drop.rs:36:9
+  --> $DIR/partial-drop.rs:38:9
    |
 LL |         let guard = Bar { foo: Foo, x: 42 };
    |             ----- has type `Bar` which is not `Send`
@@ -62,7 +62,7 @@ LL |         yield;
 LL |     });
    |     - `guard` is later dropped here
 note: required by a bound in `assert_send`
-  --> $DIR/partial-drop.rs:40:19
+  --> $DIR/partial-drop.rs:42:19
    |
 LL | fn assert_send<T: Send>(_: T) {}
    |                   ^^^^ required by this bound in `assert_send`