From 045271ccccd6c485f65ed94aeef3357a512e22fb Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Thu, 5 Dec 2024 02:58:59 +0800 Subject: [PATCH 1/6] run borrowck tests on BIDs and emit tail-expr-drop-order lints for potential violations --- compiler/rustc_borrowck/messages.ftl | 3 + compiler/rustc_borrowck/src/lib.rs | 79 +++++++++++++++---- .../rustc_borrowck/src/session_diagnostics.rs | 7 ++ compiler/rustc_mir_build/src/builder/scope.rs | 6 +- .../lint-tail-expr-drop-order-borrowck.rs | 37 +++++++++ .../lint-tail-expr-drop-order-borrowck.stderr | 40 ++++++++++ 6 files changed, 155 insertions(+), 17 deletions(-) create mode 100644 tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs create mode 100644 tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr diff --git a/compiler/rustc_borrowck/messages.ftl b/compiler/rustc_borrowck/messages.ftl index ee4b2f95cb151..c1d5af59b1b5b 100644 --- a/compiler/rustc_borrowck/messages.ftl +++ b/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 diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 88d0933d8ce8b..8d8a3c67c7b71 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -16,6 +16,7 @@ #![warn(unreachable_pub)] // tidy-alphabetical-end +use std::borrow::Cow; use std::cell::RefCell; use std::marker::PhantomData; use std::ops::{ControlFlow, Deref}; @@ -24,6 +25,7 @@ use rustc_abi::FieldIdx; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_data_structures::graph::dominators::Dominators; use rustc_hir as hir; +use rustc_hir::CRATE_HIR_ID; use rustc_hir::def_id::LocalDefId; use rustc_index::bit_set::{BitSet, MixedBitSet}; use rustc_index::{IndexSlice, IndexVec}; @@ -43,7 +45,7 @@ use rustc_mir_dataflow::move_paths::{ InitIndex, InitLocation, LookupResult, MoveData, 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}; @@ -636,9 +638,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, @@ -1007,6 +1011,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } } + fn maybe_polonius_borrows_in_scope<'s>( + &self, + location: Location, + state: &'s BorrowckDomain, + ) -> Cow<'s, BitSet> { + if let Some(polonius) = &self.polonius_output { + 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, @@ -1019,17 +1040,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { 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.maybe_polonius_borrows_in_scope(location, state); each_borrow_involving_path( self, @@ -1149,6 +1160,46 @@ 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; + + // Use polonius output if it has been enabled. + let borrows_in_scope = self.maybe_polonius_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, diff --git a/compiler/rustc_borrowck/src/session_diagnostics.rs b/compiler/rustc_borrowck/src/session_diagnostics.rs index 627444a4ce5b8..4be5d0dbf4284 100644 --- a/compiler/rustc_borrowck/src/session_diagnostics.rs +++ b/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, +} diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs index 35c98037827af..20441530a4790 100644 --- a/compiler/rustc_mir_build/src/builder/scope.rs +++ b/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(); diff --git a/tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs b/tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs new file mode 100644 index 0000000000000..1bd5655d7fe1d --- /dev/null +++ b/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 +//@ 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) {} + 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() {} diff --git a/tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr b/tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr new file mode 100644 index 0000000000000..98ef0547c9075 --- /dev/null +++ b/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 +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 + +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 + +error: aborting due to 3 previous errors + From 34edb21f0bb34916410eb92b108d84075e4abd7e Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 20 Dec 2024 01:21:51 +0800 Subject: [PATCH 2/6] apply suggestions on fn name --- compiler/rustc_borrowck/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 8d8a3c67c7b71..7eb714009d10a 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1011,12 +1011,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } } - fn maybe_polonius_borrows_in_scope<'s>( + fn borrows_in_scope<'s>( &self, location: Location, state: &'s BorrowckDomain, ) -> Cow<'s, BitSet> { 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) { @@ -1039,8 +1040,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { ) -> bool { let mut error_reported = false; - // Use polonius output if it has been enabled. - let borrows_in_scope = self.maybe_polonius_borrows_in_scope(location, state); + let borrows_in_scope = self.borrows_in_scope(location, state); each_borrow_involving_path( self, @@ -1172,7 +1172,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { let sd = AccessDepth::Drop; // Use polonius output if it has been enabled. - let borrows_in_scope = self.maybe_polonius_borrows_in_scope(location, state); + 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. From 13c7122df869f6b29799ea96599cce1fc5880376 Mon Sep 17 00:00:00 2001 From: wieDasDing <6884440+dingxiangfei2009@users.noreply.github.com> Date: Fri, 20 Dec 2024 02:08:36 +0800 Subject: [PATCH 3/6] remove an extraneous comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rémy Rakic --- compiler/rustc_borrowck/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 7eb714009d10a..df3368bdd30ae 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1171,7 +1171,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { ) { let sd = AccessDepth::Drop; - // Use polonius output if it has been enabled. let borrows_in_scope = self.borrows_in_scope(location, state); // This is a very simplified version of `Self::check_access_for_conflict`. From 197f6d8081451c5ad394601dbdd8509bbb92f446 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 5 Jan 2025 18:07:16 +0000 Subject: [PATCH 4/6] Don't create cycles by normalizing opaques defined in the body we're checking --- .../src/lint_tail_expr_drop_order.rs | 7 ++++- tests/ui/drop/lint-tail-expr-drop-order.rs | 1 - .../ui/drop/lint-tail-expr-drop-order.stderr | 27 +++++++------------ ...expr_drop_order-on-coroutine-unwind.stderr | 4 +++ ..._expr_drop_order-on-recursive-boxed-fut.rs | 13 +++++++++ 5 files changed, 33 insertions(+), 19 deletions(-) create mode 100644 tests/ui/drop/tail_expr_drop_order-on-recursive-boxed-fut.rs diff --git a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs index e5a183bc75ce5..6590702118c7f 100644 --- a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs +++ b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs @@ -351,6 +351,11 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body< { return; } + + // FIXME(typing_env): This should be able to reveal the opaques local to the + // body using the typeck results. + let typing_env = ty::TypingEnv::non_body_analysis(tcx, def_id); + // ## About BIDs in blocks ## // Track the set of blocks that contain a backwards-incompatible drop (BID) // and, for each block, the vector of locations. @@ -358,7 +363,7 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body< // We group them per-block because they tend to scheduled in the same drop ladder block. let mut bid_per_block = IndexMap::default(); let mut bid_places = UnordSet::new(); - let typing_env = ty::TypingEnv::post_analysis(tcx, def_id); + let mut ty_dropped_components = UnordMap::default(); for (block, data) in body.basic_blocks.iter_enumerated() { for (statement_index, stmt) in data.statements.iter().enumerate() { diff --git a/tests/ui/drop/lint-tail-expr-drop-order.rs b/tests/ui/drop/lint-tail-expr-drop-order.rs index b2a5db0d87133..55a2d1d3b7543 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order.rs +++ b/tests/ui/drop/lint-tail-expr-drop-order.rs @@ -17,7 +17,6 @@ impl Drop for LoudDropper { //~| NOTE: `#1` invokes this custom destructor //~| NOTE: `x` invokes this custom destructor //~| NOTE: `#1` invokes this custom destructor - //~| NOTE: `future` invokes this custom destructor //~| NOTE: `_x` invokes this custom destructor //~| NOTE: `#1` invokes this custom destructor fn drop(&mut self) { diff --git a/tests/ui/drop/lint-tail-expr-drop-order.stderr b/tests/ui/drop/lint-tail-expr-drop-order.stderr index 92afae5af6766..6ff9b7c12681d 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order.stderr +++ b/tests/ui/drop/lint-tail-expr-drop-order.stderr @@ -1,5 +1,5 @@ error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:41:15 + --> $DIR/lint-tail-expr-drop-order.rs:40:15 | LL | let x = LoudDropper; | - @@ -40,7 +40,7 @@ LL | #![deny(tail_expr_drop_order)] | ^^^^^^^^^^^^^^^^^^^^ error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:66:19 + --> $DIR/lint-tail-expr-drop-order.rs:65:19 | LL | let x = LoudDropper; | - @@ -76,7 +76,7 @@ LL | | } = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:93:7 + --> $DIR/lint-tail-expr-drop-order.rs:92:7 | LL | let x = LoudDropper; | - @@ -112,7 +112,7 @@ LL | | } = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:146:5 + --> $DIR/lint-tail-expr-drop-order.rs:145:5 | LL | let future = f(); | ------ @@ -136,19 +136,12 @@ note: `#1` invokes this custom destructor | LL | / impl Drop for LoudDropper { ... | -LL | | } - | |_^ -note: `future` invokes this custom destructor - --> $DIR/lint-tail-expr-drop-order.rs:10:1 - | -LL | / impl Drop for LoudDropper { -... | LL | | } | |_^ = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:163:14 + --> $DIR/lint-tail-expr-drop-order.rs:162:14 | LL | let x = T::default(); | - @@ -170,7 +163,7 @@ LL | } = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:177:5 + --> $DIR/lint-tail-expr-drop-order.rs:176:5 | LL | let x: Result = Ok(LoudDropper); | - @@ -206,7 +199,7 @@ LL | | } = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:221:5 + --> $DIR/lint-tail-expr-drop-order.rs:220:5 | LL | let x = LoudDropper2; | - @@ -226,7 +219,7 @@ LL | } = warning: this changes meaning in Rust 2024 = note: for more information, see note: `#1` invokes this custom destructor - --> $DIR/lint-tail-expr-drop-order.rs:194:5 + --> $DIR/lint-tail-expr-drop-order.rs:193:5 | LL | / impl Drop for LoudDropper3 { LL | | @@ -236,7 +229,7 @@ LL | | } LL | | } | |_____^ note: `x` invokes this custom destructor - --> $DIR/lint-tail-expr-drop-order.rs:206:5 + --> $DIR/lint-tail-expr-drop-order.rs:205:5 | LL | / impl Drop for LoudDropper2 { LL | | @@ -248,7 +241,7 @@ LL | | } = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages error: relative drop order changing in Rust 2024 - --> $DIR/lint-tail-expr-drop-order.rs:234:13 + --> $DIR/lint-tail-expr-drop-order.rs:233:13 | LL | LoudDropper.get() | ^^^^^^^^^^^ diff --git a/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr b/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr index d98100bc1b04c..b0f971dd5cec6 100644 --- a/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr +++ b/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr @@ -4,10 +4,14 @@ error: relative drop order changing in Rust 2024 LL | match func().await { | ^^^^^^^----- | | | + | | this value will be stored in a temporary; let us call it `#3` + | | up until Edition 2021 `#3` is dropped last but will be dropped earlier in Edition 2024 | | this value will be stored in a temporary; let us call it `#1` | | `#1` will be dropped later as of Edition 2024 | this value will be stored in a temporary; let us call it `#2` | up until Edition 2021 `#2` is dropped last but will be dropped earlier in Edition 2024 + | `__awaitee` calls a custom destructor + | `__awaitee` will be dropped later as of Edition 2024 ... LL | Err(e) => {} | - diff --git a/tests/ui/drop/tail_expr_drop_order-on-recursive-boxed-fut.rs b/tests/ui/drop/tail_expr_drop_order-on-recursive-boxed-fut.rs new file mode 100644 index 0000000000000..4a72f224d9436 --- /dev/null +++ b/tests/ui/drop/tail_expr_drop_order-on-recursive-boxed-fut.rs @@ -0,0 +1,13 @@ +//@ edition: 2021 +//@ check-pass + +// Make sure we don't cycle error when normalizing types for tail expr drop order lint. + +#![deny(tail_expr_drop_order)] + +async fn test() -> Result<(), Box> { + Box::pin(test()).await?; + Ok(()) +} + +fn main() {} From 4a099b29cdb6a7841019406f0f2b5035a1fd9a08 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 5 Jan 2025 19:11:00 +0000 Subject: [PATCH 5/6] Don't do AccessDepth::Drop for types with no drop impl --- compiler/rustc_borrowck/src/lib.rs | 16 ++++-- .../tail_expr_drop_order-on-thread-local.rs | 56 +++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 tests/ui/drop/tail_expr_drop_order-on-thread-local.rs diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index df3368bdd30ae..8b3870e5cc605 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1166,10 +1166,18 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { fn check_backward_incompatible_drop( &mut self, location: Location, - place_span: (Place<'tcx>, Span), + (place, place_span): (Place<'tcx>, Span), state: &BorrowckDomain, ) { - let sd = AccessDepth::Drop; + let tcx = self.infcx.tcx; + // If this type does not need `Drop`, then treat it like a `StorageDead`. + // This is needed because we track the borrows of refs to thread locals, + // and we'll ICE because we don't track borrows behind shared references. + let sd = if place.ty(self.body, tcx).ty.needs_drop(tcx, self.body.typing_env(tcx)) { + AccessDepth::Drop + } else { + AccessDepth::Shallow(None) + }; let borrows_in_scope = self.borrows_in_scope(location, state); @@ -1179,7 +1187,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { self, self.infcx.tcx, self.body, - (sd, place_span.0), + (sd, place), self.borrow_set, |borrow_index| borrows_in_scope.contains(borrow_index), |this, _borrow_index, borrow| { @@ -1190,7 +1198,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { this.infcx.tcx.emit_node_span_lint( TAIL_EXPR_DROP_ORDER, CRATE_HIR_ID, - place_span.1, + place_span, session_diagnostics::TailExprDropOrder { borrowed }, ); // We may stop at the first case diff --git a/tests/ui/drop/tail_expr_drop_order-on-thread-local.rs b/tests/ui/drop/tail_expr_drop_order-on-thread-local.rs new file mode 100644 index 0000000000000..e38175fd1b653 --- /dev/null +++ b/tests/ui/drop/tail_expr_drop_order-on-thread-local.rs @@ -0,0 +1,56 @@ +//@ check-pass + +#![feature(thread_local)] +#![deny(tail_expr_drop_order)] + +use std::marker::PhantomData; +use std::ops::{Deref, DerefMut}; + +pub struct Global; + +#[thread_local] +static REENTRANCY_STATE: State = State { marker: PhantomData, controller: Global }; + +pub struct Token(PhantomData<*mut ()>); + +pub fn with_mut(f: impl FnOnce(&mut Token) -> T) -> T { + f(&mut REENTRANCY_STATE.borrow_mut()) +} + +pub struct State { + marker: PhantomData<*mut ()>, + controller: T, +} + +impl State { + pub fn borrow_mut(&self) -> TokenMut<'_, T> { + todo!() + } +} + +pub struct TokenMut<'a, T: ?Sized = Global> { + state: &'a State, + token: Token, +} + +impl Deref for TokenMut<'_, T> { + type Target = Token; + + fn deref(&self) -> &Self::Target { + todo!() + } +} + +impl DerefMut for TokenMut<'_, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + todo!() + } +} + +impl Drop for TokenMut<'_, T> { + fn drop(&mut self) { + todo!() + } +} + +fn main() {} From c55eefe8bc51f302cfc89d375198ca7211d4709b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 6 Jan 2025 03:25:45 +0000 Subject: [PATCH 6/6] Try to explain borrow for tail expr temporary drop order change in 2024 --- compiler/rustc_borrowck/messages.ftl | 5 ++- .../src/diagnostics/explain_borrow.rs | 18 ++++----- .../rustc_borrowck/src/diagnostics/mod.rs | 11 +++--- .../src/diagnostics/region_name.rs | 4 +- compiler/rustc_borrowck/src/lib.rs | 19 +++++++--- ...hod_1.ElaborateDrops.after.panic-abort.mir | 1 + ...od_1.ElaborateDrops.after.panic-unwind.mir | 1 + .../lint-tail-expr-drop-order-borrowck.rs | 26 ++++++++++--- .../lint-tail-expr-drop-order-borrowck.stderr | 38 ++++++++++++------- 9 files changed, 81 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_borrowck/messages.ftl b/compiler/rustc_borrowck/messages.ftl index c1d5af59b1b5b..ada20e5c614f8 100644 --- a/compiler/rustc_borrowck/messages.ftl +++ b/compiler/rustc_borrowck/messages.ftl @@ -213,8 +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_tail_expr_drop_order = relative drop order changing in Rust 2024 + .label = this temporary value will be dropped at the end of the block + .note = consider using a `let` binding to ensure the value will live long enough borrowck_ty_no_impl_copy = {$is_partial_move -> diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index 48f28f3f1de95..a52dc447d7685 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -5,7 +5,7 @@ use std::assert_matches::assert_matches; -use rustc_errors::{Applicability, Diag}; +use rustc_errors::{Applicability, Diag, EmissionGuarantee}; use rustc_hir as hir; use rustc_hir::intravisit::Visitor; use rustc_infer::infer::NllRegionVariableOrigin; @@ -61,10 +61,10 @@ impl<'tcx> BorrowExplanation<'tcx> { pub(crate) fn is_explained(&self) -> bool { !matches!(self, BorrowExplanation::Unexplained) } - pub(crate) fn add_explanation_to_diagnostic( + pub(crate) fn add_explanation_to_diagnostic( &self, cx: &MirBorrowckCtxt<'_, '_, 'tcx>, - err: &mut Diag<'_>, + err: &mut Diag<'_, G>, borrow_desc: &str, borrow_span: Option, multiple_borrow_span: Option<(Span, Span)>, @@ -346,10 +346,10 @@ impl<'tcx> BorrowExplanation<'tcx> { } } - fn add_object_lifetime_default_note( + fn add_object_lifetime_default_note( &self, tcx: TyCtxt<'tcx>, - err: &mut Diag<'_>, + err: &mut Diag<'_, G>, unsize_ty: Ty<'tcx>, ) { if let ty::Adt(def, args) = unsize_ty.kind() { @@ -403,9 +403,9 @@ impl<'tcx> BorrowExplanation<'tcx> { } } - fn add_lifetime_bound_suggestion_to_diagnostic( + fn add_lifetime_bound_suggestion_to_diagnostic( &self, - err: &mut Diag<'_>, + err: &mut Diag<'_, G>, category: &ConstraintCategory<'tcx>, span: Span, region_name: &RegionName, @@ -432,14 +432,14 @@ impl<'tcx> BorrowExplanation<'tcx> { } } -fn suggest_rewrite_if_let( +fn suggest_rewrite_if_let( tcx: TyCtxt<'_>, expr: &hir::Expr<'_>, pat: &str, init: &hir::Expr<'_>, conseq: &hir::Expr<'_>, alt: Option<&hir::Expr<'_>>, - err: &mut Diag<'_>, + err: &mut Diag<'_, G>, ) { let source_map = tcx.sess.source_map(); err.span_note( diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 0286ea6cd9dca..d9d9ea75994f1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use rustc_abi::{FieldIdx, VariantIdx}; use rustc_data_structures::fx::FxIndexMap; -use rustc_errors::{Applicability, Diag, MultiSpan}; +use rustc_errors::{Applicability, Diag, EmissionGuarantee, MultiSpan}; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::{self as hir, CoroutineKind, LangItem}; use rustc_index::IndexSlice; @@ -626,9 +626,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { /// Add a note to region errors and borrow explanations when higher-ranked regions in predicates /// implicitly introduce an "outlives `'static`" constraint. - fn add_placeholder_from_predicate_note( + fn add_placeholder_from_predicate_note( &self, - err: &mut Diag<'_>, + err: &mut Diag<'_, G>, path: &[OutlivesConstraint<'tcx>], ) { let predicate_span = path.iter().find_map(|constraint| { @@ -651,9 +651,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { /// Add a label to region errors and borrow explanations when outlives constraints arise from /// proving a type implements `Sized` or `Copy`. - fn add_sized_or_copy_bound_info( + fn add_sized_or_copy_bound_info( &self, - err: &mut Diag<'_>, + err: &mut Diag<'_, G>, blamed_category: ConstraintCategory<'tcx>, path: &[OutlivesConstraint<'tcx>], ) { @@ -1042,6 +1042,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { kind, }; } + normal_ret } diff --git a/compiler/rustc_borrowck/src/diagnostics/region_name.rs b/compiler/rustc_borrowck/src/diagnostics/region_name.rs index bdb880b2bced3..9349b46ec5b0c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_name.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_name.rs @@ -5,7 +5,7 @@ use std::fmt::{self, Display}; use std::iter; use rustc_data_structures::fx::IndexEntry; -use rustc_errors::Diag; +use rustc_errors::{Diag, EmissionGuarantee}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_middle::ty::print::RegionHighlightMode; @@ -108,7 +108,7 @@ impl RegionName { } } - pub(crate) fn highlight_region_name(&self, diag: &mut Diag<'_>) { + pub(crate) fn highlight_region_name(&self, diag: &mut Diag<'_, G>) { match &self.source { RegionNameSource::NamedLateParamRegion(span) | RegionNameSource::NamedEarlyParamRegion(span) => { diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 8b3870e5cc605..f90f75141026f 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -24,6 +24,7 @@ use std::ops::{ControlFlow, Deref}; use rustc_abi::FieldIdx; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_data_structures::graph::dominators::Dominators; +use rustc_errors::LintDiagnostic; use rustc_hir as hir; use rustc_hir::CRATE_HIR_ID; use rustc_hir::def_id::LocalDefId; @@ -1192,17 +1193,25 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { |borrow_index| borrows_in_scope.contains(borrow_index), |this, _borrow_index, borrow| { if matches!(borrow.kind, BorrowKind::Fake(_)) { - return Control::Continue; + return ControlFlow::Continue(()); } let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span(); - this.infcx.tcx.emit_node_span_lint( + let explain = this.explain_why_borrow_contains_point( + location, + borrow, + Some((WriteKind::StorageDeadOrDrop, place)), + ); + this.infcx.tcx.node_span_lint( TAIL_EXPR_DROP_ORDER, CRATE_HIR_ID, - place_span, - session_diagnostics::TailExprDropOrder { borrowed }, + borrowed, + |diag| { + session_diagnostics::TailExprDropOrder { borrowed }.decorate_lint(diag); + explain.add_explanation_to_diagnostic(&this, diag, "", None, None); + }, ); // We may stop at the first case - Control::Break + ControlFlow::Break(()) }, ); } diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir index e9bbe30bd774f..ee6e16d20fd8d 100644 --- a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir +++ b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir @@ -74,6 +74,7 @@ fn method_1(_1: Guard) -> () { bb7: { backward incompatible drop(_2); + backward incompatible drop(_4); backward incompatible drop(_5); goto -> bb21; } diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir index e9bbe30bd774f..ee6e16d20fd8d 100644 --- a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir +++ b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir @@ -74,6 +74,7 @@ fn method_1(_1: Guard) -> () { bb7: { backward incompatible drop(_2); + backward incompatible drop(_4); backward incompatible drop(_5); goto -> bb21; } diff --git a/tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs b/tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs index 1bd5655d7fe1d..6f64d83f8a0c3 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs +++ b/tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs @@ -7,18 +7,20 @@ fn should_lint_with_potential_borrowck_err() { let _ = { String::new().as_str() }.len(); - //~^ ERROR: a temporary value will be dropped here + //~^ ERROR: relative drop order changing //~| WARN: this changes meaning in Rust 2024 - //~| NOTE: consider using a `let` binding + //~| NOTE: this temporary value will be dropped at the end of the block + //~| borrow later used by call //~| 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 + //~^ ERROR: relative drop order changing //~| WARN: this changes meaning in Rust 2024 - //~| NOTE: consider using a `let` binding + //~| NOTE: this temporary value will be dropped at the end of the block + //~| borrow later used by call //~| NOTE: for more information, see } @@ -27,11 +29,23 @@ fn should_lint_with_big_block() { fn f(_: T) {} f({ &mut || 0 - //~^ ERROR: a temporary value will be dropped here + //~^ ERROR: relative drop order changing //~| WARN: this changes meaning in Rust 2024 - //~| NOTE: consider using a `let` binding + //~| NOTE: this temporary value will be dropped at the end of the block + //~| borrow later used here //~| NOTE: for more information, see }) } +fn another_temp_that_is_copy_in_arg() { + fn f() {} + fn g(_: &()) {} + g({ &f() }); + //~^ ERROR: relative drop order changing + //~| WARN: this changes meaning in Rust 2024 + //~| NOTE: this temporary value will be dropped at the end of the block + //~| borrow later used by call + //~| NOTE: for more information, see +} + fn main() {} diff --git a/tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr b/tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr index 98ef0547c9075..a55e366dd0be1 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr +++ b/tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr @@ -1,10 +1,10 @@ -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 +error: relative drop order changing in Rust 2024 + --> $DIR/lint-tail-expr-drop-order-borrowck.rs:9:15 | LL | let _ = { String::new().as_str() }.len(); - | ------------- ^ + | ^^^^^^^^^^^^^ --- borrow later used by call | | - | 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 { .. }` + | this temporary value will be dropped at the end of the block | = warning: this changes meaning in Rust 2024 = note: for more information, see @@ -14,27 +14,39 @@ note: the lint level is defined here 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 +error: relative drop order changing in Rust 2024 + --> $DIR/lint-tail-expr-drop-order-borrowck.rs:19:16 | LL | f(unsafe { String::new().as_str() }.len()); - | ------------- ^ + | ^^^^^^^^^^^^^ --- borrow later used by call | | - | 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 { .. }` + | this temporary value will be dropped at the end of the block | = warning: this changes meaning in Rust 2024 = note: for more information, see -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 +error: relative drop order changing in Rust 2024 + --> $DIR/lint-tail-expr-drop-order-borrowck.rs:31:9 | 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 { .. }` + | this temporary value will be dropped at the end of the block + | borrow later used here | = warning: this changes meaning in Rust 2024 = note: for more information, see -error: aborting due to 3 previous errors +error: relative drop order changing in Rust 2024 + --> $DIR/lint-tail-expr-drop-order-borrowck.rs:43:9 + | +LL | g({ &f() }); + | - ^^^^ this temporary value will be dropped at the end of the block + | | + | borrow later used by call + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see + +error: aborting due to 4 previous errors