Skip to content

const checking: properly compute the set of transient locals #129508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 32 additions & 18 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations.

use std::assert_matches::assert_matches;
use std::borrow::Cow;
use std::mem;
use std::ops::Deref;

Expand All @@ -15,6 +16,8 @@ use rustc_middle::mir::*;
use rustc_middle::span_bug;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt, TypeVisitableExt};
use rustc_mir_dataflow::impls::MaybeStorageLive;
use rustc_mir_dataflow::storage::always_storage_live_locals;
use rustc_mir_dataflow::Analysis;
use rustc_span::{sym, Span, Symbol, DUMMY_SP};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
Expand Down Expand Up @@ -188,8 +191,9 @@ pub struct Checker<'mir, 'tcx> {
/// The span of the current statement.
span: Span,

/// A set that stores for each local whether it has a `StorageDead` for it somewhere.
local_has_storage_dead: Option<BitSet<Local>>,
/// A set that stores for each local whether it is "transient", i.e. guaranteed to be dead
/// when this MIR body returns.
transient_locals: Option<BitSet<Local>>,

error_emitted: Option<ErrorGuaranteed>,
secondary_errors: Vec<Diag<'tcx>>,
Expand All @@ -209,7 +213,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
span: ccx.body.span,
ccx,
qualifs: Default::default(),
local_has_storage_dead: None,
transient_locals: None,
error_emitted: None,
secondary_errors: Vec::new(),
}
Expand Down Expand Up @@ -264,23 +268,33 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
}
}

fn local_has_storage_dead(&mut self, local: Local) -> bool {
fn local_is_transient(&mut self, local: Local) -> bool {
let ccx = self.ccx;
self.local_has_storage_dead
self.transient_locals
.get_or_insert_with(|| {
struct StorageDeads {
locals: BitSet<Local>,
}
impl<'tcx> Visitor<'tcx> for StorageDeads {
fn visit_statement(&mut self, stmt: &Statement<'tcx>, _: Location) {
if let StatementKind::StorageDead(l) = stmt.kind {
self.locals.insert(l);
}
// A local is "transient" if it is guaranteed dead at all `Return`.
// So first compute the say of "maybe live" locals at each program point.
let always_live_locals = &always_storage_live_locals(&ccx.body);
let mut maybe_storage_live =
MaybeStorageLive::new(Cow::Borrowed(always_live_locals))
.into_engine(ccx.tcx, &ccx.body)
.iterate_to_fixpoint()
.into_results_cursor(&ccx.body);

// And then check all `Return` in the MIR, and if a local is "maybe live" at a
// `Return` then it is definitely not transient.
let mut transient = BitSet::new_filled(ccx.body.local_decls.len());
// Make sure to only visit reachable blocks, the dataflow engine can ICE otherwise.
for (bb, data) in traversal::reachable(&ccx.body) {
if matches!(data.terminator().kind, TerminatorKind::Return) {
let location = ccx.body.terminator_loc(bb);
maybe_storage_live.seek_after_primary_effect(location);
// If a local may be live here, it is definitely not transient.
transient.subtract(maybe_storage_live.get());
}
}
let mut v = StorageDeads { locals: BitSet::new_empty(ccx.body.local_decls.len()) };
v.visit_body(ccx.body);
v.locals

transient
})
.contains(local)
}
Expand Down Expand Up @@ -375,7 +389,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
// `StorageDead` in every control flow path leading to a `return` terminator.
// The good news is that interning will detect if any unexpected mutable
// pointer slips through.
if place.is_indirect() || self.local_has_storage_dead(place.local) {
if place.is_indirect() || self.local_is_transient(place.local) {
self.check_op(ops::TransientMutBorrow(kind));
} else {
self.check_op(ops::MutBorrow(kind));
Expand Down Expand Up @@ -526,7 +540,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
// `StorageDead` in every control flow path leading to a `return` terminator.
// The good news is that interning will detect if any unexpected mutable
// pointer slips through.
if self.local_has_storage_dead(place.local) {
if self.local_is_transient(place.local) {
self.check_op(ops::TransientCellBorrow);
} else {
self.check_op(ops::CellBorrow);
Expand Down
Loading