From 214ec38cb95e269c67fd18d91c8df67ded47b12f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 20 Aug 2026 09:27:23 +1000 Subject: [PATCH 1/2] Improve live-drop precision `State::join` uses a short-circuiting `||` when it should use `|`. As a result: - `self.borrow` sometimes is missing some bits it should have; - which means `self.state.borrow.contains(local)` fails when it shouldn't; - which means `!self.state.borrow.contains(local)` succeeds when it shouldn't; - which means `self.state.qualif.remove(local);` runs when it shouldn't; In practice, this means that some analysis information gets lost across basic block boundaries, and the presence/absence of a simple branch or function call or loop can change whether code is accepted or rejected. Not good! This bug has been present since this code was first added five years ago in #90214. This commit fixes that bug. But with the operator change alone we get this compile error when building std: ``` error[E0493]: destructor of `MaybeDangling

` cannot be evaluated at compile-time --> library/core/src/mem/maybe_dangling.rs:102:29 | 102 | pub const fn into_inner(self) -> P | ^^^^ the destructor for this type cannot be evaluated in constant functions ... 111 | } | - value is dropped here ``` This is a case where a basic block boundary was preventing this code from being rejected. We don't want to reject code that was previously accepted due to a bug. Fortunately we can instead accept some code that used to be rejected, by removing the qualif unconditionally in `TransferFunction::visit_operand`. The existing behaviour is conservative and the existing comment already says the check "might not be strictly necessary" and was only present to match an older implementation. Removing the qualif unconditionally makes the vanilla const drop check more precise, giving it some of the precision that the `const_precise_live_drops` feature provides (just some whole move cases, not the partial move cases), without requiring that feature's extra machinery. This can be seen in the `issue-65394.rs` test, which now compiles without needing the feature. --- .../src/check_consts/resolver.rs | 11 ++++------- tests/ui/consts/const-eval/issue-65394.rs | 16 +++++++--------- .../consts/const-eval/issue-65394.stock.stderr | 16 ---------------- 3 files changed, 11 insertions(+), 32 deletions(-) delete mode 100644 tests/ui/consts/const-eval/issue-65394.stock.stderr diff --git a/compiler/rustc_const_eval/src/check_consts/resolver.rs b/compiler/rustc_const_eval/src/check_consts/resolver.rs index 4fc8c62ccf80a..74f84f64b25d7 100644 --- a/compiler/rustc_const_eval/src/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/check_consts/resolver.rs @@ -140,12 +140,8 @@ where if let mir::Operand::Move(place) = operand && let Some(local) = place.as_local() { - // For backward compatibility with the MaybeMutBorrowedLocals used in an earlier - // implementation we retain qualif if a local had been borrowed before. This might - // not be strictly necessary since the local is no longer initialized. - if !self.state.borrow.contains(local) { - self.state.qualif.remove(local); - } + // The local is no longer initialized so we can remove the qualif. + self.state.qualif.remove(local); } } @@ -319,7 +315,8 @@ impl DebugWithContext for State { impl JoinSemiLattice for State { fn join(&mut self, other: &Self) -> bool { - self.qualif.join(&other.qualif) || self.borrow.join(&other.borrow) + // Use `|` not `||` here; we don't want short-circuiting. + self.qualif.join(&other.qualif) | self.borrow.join(&other.borrow) } } diff --git a/tests/ui/consts/const-eval/issue-65394.rs b/tests/ui/consts/const-eval/issue-65394.rs index 2304dbebc7ef2..77ea60724eb15 100644 --- a/tests/ui/consts/const-eval/issue-65394.rs +++ b/tests/ui/consts/const-eval/issue-65394.rs @@ -1,15 +1,13 @@ -//@ revisions: stock precise_drops -//@[precise_drops] check-pass +//@ check-pass -// This test originated from #65394. We conservatively assume that `x` is still `LiveDrop` even -// after it has been moved because a mutable reference to it exists at some point in the const body. -// -// With `&mut` in `const` being stable, this surprising behavior is now observable. -// `const_precise_live_drops` fixes that. -#![cfg_attr(precise_drops, feature(const_precise_live_drops))] +// This test originated from #65394. It used to fail to compile without +// `feature(const_precise_live_drops)` because we conservatively assumed that `x` is still +// `LiveDrop` even after it has been moved because a mutable reference to it exists at some point +// in the const body. With `&mut` in `const` being stable, this surprising behavior was observable. +// Later fixes to drop handling fixed that (#161627). const _: Vec = { - let mut x = Vec::::new(); //[stock]~ ERROR destructor of + let mut x = Vec::::new(); let r = &mut x; let y = x; y diff --git a/tests/ui/consts/const-eval/issue-65394.stock.stderr b/tests/ui/consts/const-eval/issue-65394.stock.stderr deleted file mode 100644 index 514d454df3dc0..0000000000000 --- a/tests/ui/consts/const-eval/issue-65394.stock.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error[E0493]: destructor of `Vec` cannot be evaluated at compile-time - --> $DIR/issue-65394.rs:12:9 - | -LL | let mut x = Vec::::new(); - | ^^^^^ the destructor for this type cannot be evaluated in constants -... -LL | }; - | - value is dropped here - | - = note: see issue #133214 for more information - = help: add `#![feature(const_destruct)]` to the crate attributes to enable - = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0493`. From 8c4981605a0f03d1cbe9d660298e4c09ba409267 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 20 Aug 2026 10:10:00 +1000 Subject: [PATCH 2/2] Remove `State::borrow` The last commit removed its only read, so it can be removed. With it gone, `State` has a single field and so can be replaced with `MixedBitSet`. --- .../src/check_consts/resolver.rs | 116 ++++-------------- 1 file changed, 21 insertions(+), 95 deletions(-) diff --git a/compiler/rustc_const_eval/src/check_consts/resolver.rs b/compiler/rustc_const_eval/src/check_consts/resolver.rs index 74f84f64b25d7..5ac40f52be8df 100644 --- a/compiler/rustc_const_eval/src/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/check_consts/resolver.rs @@ -2,7 +2,6 @@ //! //! This contains the dataflow analysis used to track `Qualif`s on complex control-flow graphs. -use std::fmt; use std::marker::PhantomData; use rustc_index::bit_set::MixedBitSet; @@ -10,8 +9,7 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{ self, BasicBlock, CallReturnPlaces, Local, Location, Statement, StatementKind, }; -use rustc_mir_dataflow::fmt::DebugWithContext; -use rustc_mir_dataflow::{Analysis, JoinSemiLattice}; +use rustc_mir_dataflow::Analysis; use super::{ConstCx, Qualif, qualifs}; @@ -24,7 +22,7 @@ use super::{ConstCx, Qualif, qualifs}; /// borrowed place must contain the qualif. struct TransferFunction<'mir, 'tcx, Q> { ccx: &'mir ConstCx<'mir, 'tcx>, - state: &'mir mut State, + state: &'mir mut MixedBitSet, _qualif: PhantomData, } @@ -32,18 +30,17 @@ impl<'mir, 'tcx, Q> TransferFunction<'mir, 'tcx, Q> where Q: Qualif, { - fn new(ccx: &'mir ConstCx<'mir, 'tcx>, state: &'mir mut State) -> Self { + fn new(ccx: &'mir ConstCx<'mir, 'tcx>, state: &'mir mut MixedBitSet) -> Self { TransferFunction { ccx, state, _qualif: PhantomData } } fn initialize_state(&mut self) { - self.state.qualif.clear(); - self.state.borrow.clear(); + self.state.clear(); for arg in self.ccx.body.args_iter() { let arg_ty = self.ccx.body.local_decls[arg].ty; if Q::in_any_value_of_ty(self.ccx, arg_ty) { - self.state.qualif.insert(arg); + self.state.insert(arg); } } } @@ -63,7 +60,7 @@ where match (value, place.as_ref()) { (true, mir::PlaceRef { local, .. }) => { - self.state.qualif.insert(local); + self.state.insert(local); } // For now, we do not clear the qualif if a local is overwritten in full by @@ -71,7 +68,7 @@ where // with aggregates where we overwrite all fields with assignments, which would not // get this feature. (false, mir::PlaceRef { local: _, projection: &[] }) => { - // self.state.qualif.remove(*local); + // self.state.remove(*local); } _ => {} @@ -141,7 +138,7 @@ where && let Some(local) = place.as_local() { // The local is no longer initialized so we can remove the qualif. - self.state.qualif.remove(local); + self.state.remove(local); } } @@ -151,8 +148,7 @@ where rvalue: &mir::Rvalue<'tcx>, location: Location, ) { - let qualif = - qualifs::in_rvalue::(self.ccx, &mut |l| self.state.qualif.contains(l), rvalue); + let qualif = qualifs::in_rvalue::(self.ccx, &mut |l| self.state.contains(l), rvalue); if !place.is_indirect() { self.assign_qualif_direct(place, qualif); } @@ -170,8 +166,7 @@ where if !borrowed_place.is_indirect() && self.address_of_allows_mutation() { let place_ty = borrowed_place.ty(self.ccx.body, self.ccx.tcx).ty; if Q::in_any_value_of_ty(self.ccx, place_ty) { - self.state.qualif.insert(borrowed_place.local); - self.state.borrow.insert(borrowed_place.local); + self.state.insert(borrowed_place.local); } } } @@ -181,8 +176,7 @@ where { let place_ty = borrowed_place.ty(self.ccx.body, self.ccx.tcx).ty; if Q::in_any_value_of_ty(self.ccx, place_ty) { - self.state.qualif.insert(borrowed_place.local); - self.state.borrow.insert(borrowed_place.local); + self.state.insert(borrowed_place.local); } } } @@ -194,8 +188,7 @@ where && (mutability.is_mut() || !target.is_freeze(self.ccx.tcx, self.ccx.typing_env)) { if Q::in_any_value_of_ty(self.ccx, *target) { - self.state.qualif.insert(borrowed_place.local); - self.state.borrow.insert(borrowed_place.local); + self.state.insert(borrowed_place.local); } } } @@ -216,8 +209,7 @@ where fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match statement.kind { StatementKind::StorageDead(local) => { - self.state.qualif.remove(local); - self.state.borrow.remove(local); + self.state.remove(local); } _ => self.super_statement(statement, location), } @@ -247,92 +239,26 @@ where FlowSensitiveAnalysis { ccx, _qualif: PhantomData } } - fn transfer_function(&self, state: &'mir mut State) -> TransferFunction<'mir, 'tcx, Q> { + fn transfer_function( + &self, + state: &'mir mut MixedBitSet, + ) -> TransferFunction<'mir, 'tcx, Q> { TransferFunction::::new(self.ccx, state) } } -#[derive(Debug, PartialEq, Eq)] -/// The state for the `FlowSensitiveAnalysis` dataflow analysis. This domain is likely homogeneous, -/// and has a big size, so we use a bitset that can be sparse (c.f. issue #134404). -pub(super) struct State { - /// Describes whether a local contains qualif. - pub qualif: MixedBitSet, - /// Describes whether a local's address escaped and it might become qualified as a result an - /// indirect mutation. - pub borrow: MixedBitSet, -} - -impl Clone for State { - fn clone(&self) -> Self { - State { qualif: self.qualif.clone(), borrow: self.borrow.clone() } - } - - // Data flow engine when possible uses `clone_from` for domain values. - // Providing an implementation will avoid some intermediate memory allocations. - fn clone_from(&mut self, other: &Self) { - self.qualif.clone_from(&other.qualif); - self.borrow.clone_from(&other.borrow); - } -} - -impl State { - #[inline] - pub(super) fn contains(&self, local: Local) -> bool { - self.qualif.contains(local) - } -} - -impl DebugWithContext for State { - fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("qualif: ")?; - self.qualif.fmt_with(ctxt, f)?; - f.write_str(" borrow: ")?; - self.borrow.fmt_with(ctxt, f)?; - Ok(()) - } - - fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if self == old { - return Ok(()); - } - - if self.qualif != old.qualif { - f.write_str("qualif: ")?; - self.qualif.fmt_diff_with(&old.qualif, ctxt, f)?; - f.write_str("\n")?; - } - - if self.borrow != old.borrow { - f.write_str("borrow: ")?; - self.borrow.fmt_diff_with(&old.borrow, ctxt, f)?; - f.write_str("\n")?; - } - - Ok(()) - } -} - -impl JoinSemiLattice for State { - fn join(&mut self, other: &Self) -> bool { - // Use `|` not `||` here; we don't want short-circuiting. - self.qualif.join(&other.qualif) | self.borrow.join(&other.borrow) - } -} - impl<'tcx, Q> Analysis<'tcx> for FlowSensitiveAnalysis<'_, 'tcx, Q> where Q: Qualif, { - type Domain = State; + /// Describes whether a local contains qualif. This domain is likely homogeneous, and has a big + /// size, so we use a bitset that can be sparse (c.f. issue #134404). + type Domain = MixedBitSet; const NAME: &'static str = Q::ANALYSIS_NAME; fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { - State { - qualif: MixedBitSet::new_empty(body.local_decls.len()), - borrow: MixedBitSet::new_empty(body.local_decls.len()), - } + MixedBitSet::new_empty(body.local_decls.len()) } fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut Self::Domain) {