Auto merge of #134523 - dingxiangfei2009:issue-130836-attempt-2, r=<try>

Run borrowck tests on BIDs and emit tail-expr-drop-order lints for violations

Fix #132861

r? `@nikomatsakis`
cc `@compiler-errors`

This patch enlarges the scope where the `tail-expr-drop-order` lint applies, so that all locals involved in tail expressions are inspected. This is necessary to run borrow-checking to capture the cases where it used to compile under Edition 2021 but is not going to pass borrow-checking from Edition 2024 onwards.

The way it works is to inspect each BID against the set of borrows that are still live. If the local involved in BID has a borrow index which happens to be live as well at the location of this BID statement, in the future this will be a borrow-checking violation. The lint will fire in this case.

Author: bors

compiler/rustc_borrowck/messages.ftl

@@ -213,6 +213,9 @@ borrowck_suggest_create_fresh_reborrow =
 borrowck_suggest_iterate_over_slice =
     consider iterating over a slice of the `{$ty}`'s content to avoid moving into the `for` loop
 
+borrowck_tail_expr_drop_order = a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
+    .label = consider using a `let` binding to create a longer lived value; or replacing the `{"{"} .. {"}"}` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe {"{"} .. {"}"}`
+
 borrowck_ty_no_impl_copy =
     {$is_partial_move ->
         [true] partial move

compiler/rustc_borrowck/src/lib.rs

@@ -15,6 +15,7 @@
 #![warn(unreachable_pub)]
 // tidy-alphabetical-end
 
+use std::borrow::Cow;
 use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::marker::PhantomData;
@@ -24,8 +25,8 @@ use rustc_abi::FieldIdx;
 use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
 use rustc_data_structures::graph::dominators::Dominators;
 use rustc_errors::Diag;
-use rustc_hir as hir;
 use rustc_hir::def_id::LocalDefId;
+use rustc_hir::{self as hir, CRATE_HIR_ID};
 use rustc_index::bit_set::{BitSet, MixedBitSet};
 use rustc_index::{IndexSlice, IndexVec};
 use rustc_infer::infer::{
@@ -44,7 +45,7 @@ use rustc_mir_dataflow::move_paths::{
     InitIndex, InitLocation, LookupResult, MoveData, MoveOutIndex, MovePathIndex,
 };
 use rustc_mir_dataflow::{Analysis, EntryStates, Results, ResultsVisitor, visit_results};
-use rustc_session::lint::builtin::UNUSED_MUT;
+use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT};
 use rustc_span::{Span, Symbol};
 use smallvec::SmallVec;
 use tracing::{debug, instrument};
@@ -622,9 +623,11 @@ impl<'a, 'tcx> ResultsVisitor<'a, 'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<
             | StatementKind::Coverage(..)
             // These do not actually affect borrowck
             | StatementKind::ConstEvalCounter
-            // This do not affect borrowck
-            | StatementKind::BackwardIncompatibleDropHint { .. }
             | StatementKind::StorageLive(..) => {}
+            // This does not affect borrowck
+            StatementKind::BackwardIncompatibleDropHint { place, reason: BackwardIncompatibleDropReason::Edition2024 } => {
+                self.check_backward_incompatible_drop(location, (**place, span), state);
+            }
             StatementKind::StorageDead(local) => {
                 self.access_place(
                     location,
@@ -994,6 +997,24 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
         }
     }
 
+    fn borrows_in_scope<'s>(
+        &self,
+        location: Location,
+        state: &'s BorrowckDomain,
+    ) -> Cow<'s, BitSet<BorrowIndex>> {
+        if let Some(polonius) = &self.polonius_output {
+            // Use polonius output if it has been enabled.
+            let location = self.location_table.start_index(location);
+            let mut polonius_output = BitSet::new_empty(self.borrow_set.len());
+            for &idx in polonius.errors_at(location) {
+                polonius_output.insert(idx);
+            }
+            Cow::Owned(polonius_output)
+        } else {
+            Cow::Borrowed(&state.borrows)
+        }
+    }
+
     #[instrument(level = "debug", skip(self, state))]
     fn check_access_for_conflict(
         &mut self,
@@ -1005,18 +1026,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
     ) -> bool {
         let mut error_reported = false;
 
-        // Use polonius output if it has been enabled.
-        let mut polonius_output;
-        let borrows_in_scope = if let Some(polonius) = &self.polonius_output {
-            let location = self.location_table.start_index(location);
-            polonius_output = BitSet::new_empty(self.borrow_set.len());
-            for &idx in polonius.errors_at(location) {
-                polonius_output.insert(idx);
-            }
-            &polonius_output
-        } else {
-            &state.borrows
-        };
+        let borrows_in_scope = self.borrows_in_scope(location, state);
 
         each_borrow_involving_path(
             self,
@@ -1136,6 +1146,45 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
         error_reported
     }
 
+    /// Through #123739, backward incompatible drops (BIDs) are introduced.
+    /// We would like to emit lints whether borrow checking fails at these future drop locations.
+    #[instrument(level = "debug", skip(self, state))]
+    fn check_backward_incompatible_drop(
+        &mut self,
+        location: Location,
+        place_span: (Place<'tcx>, Span),
+        state: &BorrowckDomain,
+    ) {
+        let sd = AccessDepth::Drop;
+
+        let borrows_in_scope = self.borrows_in_scope(location, state);
+
+        // This is a very simplified version of `Self::check_access_for_conflict`.
+        // We are here checking on BIDs and specifically still-live borrows of data involving the BIDs.
+        each_borrow_involving_path(
+            self,
+            self.infcx.tcx,
+            self.body,
+            (sd, place_span.0),
+            self.borrow_set,
+            |borrow_index| borrows_in_scope.contains(borrow_index),
+            |this, _borrow_index, borrow| {
+                if matches!(borrow.kind, BorrowKind::Fake(_)) {
+                    return Control::Continue;
+                }
+                let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span();
+                this.infcx.tcx.emit_node_span_lint(
+                    TAIL_EXPR_DROP_ORDER,
+                    CRATE_HIR_ID,
+                    place_span.1,
+                    session_diagnostics::TailExprDropOrder { borrowed },
+                );
+                // We may stop at the first case
+                Control::Break
+            },
+        );
+    }
+
     fn mutate_place(
         &mut self,
         location: Location,

compiler/rustc_borrowck/src/session_diagnostics.rs

@@ -480,3 +480,10 @@ pub(crate) struct SimdIntrinsicArgConst {
     pub arg: usize,
     pub intrinsic: String,
 }
+
+#[derive(LintDiagnostic)]
+#[diag(borrowck_tail_expr_drop_order)]
+pub(crate) struct TailExprDropOrder {
+    #[label]
+    pub borrowed: Span,
+}

compiler/rustc_mir_build/src/builder/scope.rs

@@ -1131,15 +1131,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
     /// Schedule emission of a backwards incompatible drop lint hint.
     /// Applicable only to temporary values for now.
+    #[instrument(level = "debug", skip(self))]
     pub(crate) fn schedule_backwards_incompatible_drop(
         &mut self,
         span: Span,
         region_scope: region::Scope,
         local: Local,
     ) {
-        if !self.local_decls[local].ty.has_significant_drop(self.tcx, self.typing_env()) {
-            return;
-        }
+        // Note that we are *not* gating BIDs here on whether they have significant destructor.
+        // We need to know all of them so that we can capture potential borrow-checking errors.
         for scope in self.scopes.scopes.iter_mut().rev() {
             // Since we are inserting linting MIR statement, we have to invalidate the caches
             scope.invalidate_cache();

tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs

@@ -0,0 +1,37 @@
+// Edition 2024 lint for change in drop order at tail expression
+// This lint is to capture potential borrow-checking errors
+// due to implementation of RFC 3606 <https://github.com/rust-lang/rfcs/pull/3606>
+//@ edition: 2021
+
+#![deny(tail_expr_drop_order)] //~ NOTE: the lint level is defined here
+
+fn should_lint_with_potential_borrowck_err() {
+    let _ = { String::new().as_str() }.len();
+    //~^ ERROR: a temporary value will be dropped here
+    //~| WARN: this changes meaning in Rust 2024
+    //~| NOTE: consider using a `let` binding
+    //~| NOTE: for more information, see
+}
+
+fn should_lint_with_unsafe_block() {
+    fn f(_: usize) {}
+    f(unsafe { String::new().as_str() }.len());
+    //~^ ERROR: a temporary value will be dropped here
+    //~| WARN: this changes meaning in Rust 2024
+    //~| NOTE: consider using a `let` binding
+    //~| NOTE: for more information, see
+}
+
+#[rustfmt::skip]
+fn should_lint_with_big_block() {
+    fn f<T>(_: T) {}
+    f({
+        &mut || 0
+        //~^ ERROR: a temporary value will be dropped here
+        //~| WARN: this changes meaning in Rust 2024
+        //~| NOTE: consider using a `let` binding
+        //~| NOTE: for more information, see
+    })
+}
+
+fn main() {}

tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr

@@ -0,0 +1,40 @@
+error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
+  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:9:36
+   |
+LL |     let _ = { String::new().as_str() }.len();
+   |               -------------        ^
+   |               |
+   |               consider using a `let` binding to create a longer lived value; or replacing the `{ .. }` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe { .. }`
+   |
+   = warning: this changes meaning in Rust 2024
+   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
+note: the lint level is defined here
+  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:6:9
+   |
+LL | #![deny(tail_expr_drop_order)]
+   |         ^^^^^^^^^^^^^^^^^^^^
+
+error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
+  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:18:37
+   |
+LL |     f(unsafe { String::new().as_str() }.len());
+   |                -------------        ^
+   |                |
+   |                consider using a `let` binding to create a longer lived value; or replacing the `{ .. }` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe { .. }`
+   |
+   = warning: this changes meaning in Rust 2024
+   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
+
+error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
+  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:29:17
+   |
+LL |         &mut || 0
+   |         --------^
+   |         |
+   |         consider using a `let` binding to create a longer lived value; or replacing the `{ .. }` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe { .. }`
+   |
+   = warning: this changes meaning in Rust 2024
+   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
+
+error: aborting due to 3 previous errors
+