Remove QualifResolver abstraction

This is a relic from earlier attempts at dataflow-based const validation
that attempted to do promotion at the same time. #63812 takes a
different approach: `IsNotPromotable` is no longer a `Qualif` and is
computed lazily instead of eagerly. As a result, there's no need for an
eager `TempPromotionResolver`, and we can use the only implementer of
`QualifResolver` directly instead of through a trait.
This commit is contained in:
Dylan MacKenzie 2019-10-23 12:10:08 -07:00
parent 653865658d
commit b93cdbce36
3 changed files with 112 additions and 206 deletions

View file

@ -1,21 +1,18 @@
//! Propagate `Qualif`s between locals and query the results.
//!
//! This also contains the dataflow analysis used to track `Qualif`s on complex control-flow
//! graphs.
//! This contains the dataflow analysis used to track `Qualif`s on complex control-flow graphs.
use rustc::mir::visit::Visitor;
use rustc::mir::{self, BasicBlock, Local, Location};
use rustc_index::bit_set::BitSet;
use std::cell::RefCell;
use std::marker::PhantomData;
use crate::dataflow::{self as old_dataflow, generic as dataflow};
use super::{Item, Qualif};
use self::old_dataflow::IndirectlyMutableLocals;
/// A `Visitor` that propagates qualifs between locals. This defines the transfer function of
/// `FlowSensitiveAnalysis` as well as the logic underlying `TempPromotionResolver`.
/// `FlowSensitiveAnalysis`.
///
/// This transfer does nothing when encountering an indirect assignment. Consumers should rely on
/// the `IndirectlyMutableLocals` dataflow pass to see if a `Local` may have become qualified via
@ -147,145 +144,6 @@ where
}
}
/// Types that can compute the qualifs of each local at each location in a `mir::Body`.
///
/// Code that wishes to use a `QualifResolver` must call `visit_{statement,terminator}` for each
/// statement or terminator, processing blocks in reverse post-order beginning from the
/// `START_BLOCK`. Calling code may optionally call `get` after visiting each statement or
/// terminator to query the qualification state immediately before that statement or terminator.
///
/// These conditions are much more restrictive than woud be required by `FlowSensitiveResolver`
/// alone. This is to allow a linear, on-demand `TempPromotionResolver` that can operate
/// efficiently on simple CFGs.
pub trait QualifResolver<Q> {
/// Get the qualifs of each local at the last location visited.
///
/// This takes `&mut self` so qualifs can be computed lazily.
fn get(&mut self) -> &BitSet<Local>;
/// A convenience method for `self.get().contains(local)`.
fn contains(&mut self, local: Local) -> bool {
self.get().contains(local)
}
/// Resets the resolver to the `START_BLOCK`. This allows a resolver to be reused
/// for multiple passes over a `mir::Body`.
fn reset(&mut self);
}
pub type IndirectlyMutableResults<'mir, 'tcx> =
old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>;
/// A resolver for qualifs that works on arbitrarily complex CFGs.
///
/// As soon as a `Local` becomes writable through a reference (as determined by the
/// `IndirectlyMutableLocals` dataflow pass), we must assume that it takes on all other qualifs
/// possible for its type. This is because no effort is made to track qualifs across indirect
/// assignments (e.g. `*p = x` or calls to opaque functions).
///
/// It is possible to be more precise here by waiting until an indirect assignment actually occurs
/// before marking a borrowed `Local` as qualified.
pub struct FlowSensitiveResolver<'a, 'mir, 'tcx, Q>
where
Q: Qualif,
{
location: Location,
indirectly_mutable_locals: &'a RefCell<IndirectlyMutableResults<'mir, 'tcx>>,
cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>,
qualifs_per_local: BitSet<Local>,
/// The value of `Q::in_any_value_of_ty` for each local.
qualifs_in_any_value_of_ty: BitSet<Local>,
}
impl<Q> FlowSensitiveResolver<'a, 'mir, 'tcx, Q>
where
Q: Qualif,
{
pub fn new(
_: Q,
item: &'a Item<'mir, 'tcx>,
indirectly_mutable_locals: &'a RefCell<IndirectlyMutableResults<'mir, 'tcx>>,
dead_unwinds: &BitSet<BasicBlock>,
) -> Self {
let analysis = FlowSensitiveAnalysis {
item,
_qualif: PhantomData,
};
let results =
dataflow::Engine::new(item.tcx, item.body, item.def_id, dead_unwinds, analysis)
.iterate_to_fixpoint();
let cursor = dataflow::ResultsCursor::new(item.body, results);
let mut qualifs_in_any_value_of_ty = BitSet::new_empty(item.body.local_decls.len());
for (local, decl) in item.body.local_decls.iter_enumerated() {
if Q::in_any_value_of_ty(item, decl.ty) {
qualifs_in_any_value_of_ty.insert(local);
}
}
FlowSensitiveResolver {
cursor,
indirectly_mutable_locals,
qualifs_per_local: BitSet::new_empty(item.body.local_decls.len()),
qualifs_in_any_value_of_ty,
location: Location { block: mir::START_BLOCK, statement_index: 0 },
}
}
}
impl<Q> Visitor<'tcx> for FlowSensitiveResolver<'_, '_, 'tcx, Q>
where
Q: Qualif
{
fn visit_statement(&mut self, _: &mir::Statement<'tcx>, location: Location) {
self.location = location;
}
fn visit_terminator(&mut self, _: &mir::Terminator<'tcx>, location: Location) {
self.location = location;
}
}
impl<Q> QualifResolver<Q> for FlowSensitiveResolver<'_, '_, '_, Q>
where
Q: Qualif
{
fn get(&mut self) -> &BitSet<Local> {
let mut indirectly_mutable_locals = self.indirectly_mutable_locals.borrow_mut();
indirectly_mutable_locals.seek(self.location);
self.cursor.seek_before(self.location);
self.qualifs_per_local.overwrite(indirectly_mutable_locals.get());
self.qualifs_per_local.union(self.cursor.get());
self.qualifs_per_local.intersect(&self.qualifs_in_any_value_of_ty);
&self.qualifs_per_local
}
fn contains(&mut self, local: Local) -> bool {
// No need to update the cursor if we know that `Local` cannot possibly be qualified.
if !self.qualifs_in_any_value_of_ty.contains(local) {
return false;
}
// Otherwise, return `true` if this local is qualified or was indirectly mutable at any
// point before this statement.
self.cursor.seek_before(self.location);
if self.cursor.get().contains(local) {
return true;
}
let mut indirectly_mutable_locals = self.indirectly_mutable_locals.borrow_mut();
indirectly_mutable_locals.seek(self.location);
indirectly_mutable_locals.get().contains(local)
}
fn reset(&mut self) {
self.location = Location { block: mir::START_BLOCK, statement_index: 0 };
}
}
/// The dataflow analysis used to propagate qualifs on arbitrary CFGs.
pub(super) struct FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q> {
item: &'a Item<'mir, 'tcx>,
@ -296,6 +154,13 @@ impl<'a, 'mir, 'tcx, Q> FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>
where
Q: Qualif,
{
pub(super) fn new(_: Q, item: &'a Item<'mir, 'tcx>) -> Self {
FlowSensitiveAnalysis {
item,
_qualif: PhantomData,
}
}
fn transfer_function(
&self,
state: &'a mut BitSet<Local>,

View file

@ -9,15 +9,15 @@ use rustc_target::spec::abi::Abi;
use syntax::symbol::sym;
use syntax_pos::Span;
use std::cell::RefCell;
use std::fmt;
use std::ops::Deref;
use crate::dataflow as old_dataflow;
use super::{ConstKind, Item, Qualif, is_lang_panic_fn};
use super::resolver::{FlowSensitiveResolver, IndirectlyMutableResults, QualifResolver};
use super::qualifs::{HasMutInterior, NeedsDrop};
use crate::dataflow::{self as old_dataflow, generic as dataflow};
use self::old_dataflow::IndirectlyMutableLocals;
use super::ops::{self, NonConstOp};
use super::qualifs::{HasMutInterior, NeedsDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{ConstKind, Item, Qualif, is_lang_panic_fn};
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum CheckOpResult {
@ -26,9 +26,75 @@ pub enum CheckOpResult {
Allowed,
}
pub type IndirectlyMutableResults<'mir, 'tcx> =
old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>;
struct QualifCursor<'a, 'mir, 'tcx, Q: Qualif> {
cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>,
in_any_value_of_ty: BitSet<Local>,
}
impl<Q: Qualif> QualifCursor<'a, 'mir, 'tcx, Q> {
pub fn new(
q: Q,
item: &'a Item<'mir, 'tcx>,
dead_unwinds: &BitSet<BasicBlock>,
) -> Self {
let analysis = FlowSensitiveAnalysis::new(q, item);
let results =
dataflow::Engine::new(item.tcx, item.body, item.def_id, dead_unwinds, analysis)
.iterate_to_fixpoint();
let cursor = dataflow::ResultsCursor::new(item.body, results);
let mut in_any_value_of_ty = BitSet::new_empty(item.body.local_decls.len());
for (local, decl) in item.body.local_decls.iter_enumerated() {
if Q::in_any_value_of_ty(item, decl.ty) {
in_any_value_of_ty.insert(local);
}
}
QualifCursor {
cursor,
in_any_value_of_ty,
}
}
}
pub struct Qualifs<'a, 'mir, 'tcx> {
has_mut_interior: FlowSensitiveResolver<'a, 'mir, 'tcx, HasMutInterior>,
needs_drop: FlowSensitiveResolver<'a, 'mir, 'tcx, NeedsDrop>,
has_mut_interior: QualifCursor<'a, 'mir, 'tcx, HasMutInterior>,
needs_drop: QualifCursor<'a, 'mir, 'tcx, NeedsDrop>,
indirectly_mutable: IndirectlyMutableResults<'mir, 'tcx>,
}
impl Qualifs<'a, 'mir, 'tcx> {
fn indirectly_mutable(&mut self, local: Local, location: Location) -> bool {
self.indirectly_mutable.seek(location);
self.indirectly_mutable.get().contains(local)
}
/// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary
fn needs_drop_lazy_seek(&mut self, local: Local, location: Location) -> bool {
if !self.needs_drop.in_any_value_of_ty.contains(local) {
return false;
}
self.needs_drop.cursor.seek_before(location);
self.needs_drop.cursor.get().contains(local)
|| self.indirectly_mutable(local, location)
}
/// Returns `true` if `local` is `HasMutInterior`, but requires the `has_mut_interior` and
/// `indirectly_mutable` cursors to be updated beforehand.
fn has_mut_interior_eager_seek(&self, local: Local) -> bool {
if !self.has_mut_interior.in_any_value_of_ty.contains(local) {
return false;
}
self.has_mut_interior.cursor.get().contains(local)
|| self.indirectly_mutable.get().contains(local)
}
}
pub struct Validator<'a, 'mir, 'tcx> {
@ -63,53 +129,43 @@ impl Deref for Validator<'_, 'mir, 'tcx> {
}
}
pub fn compute_indirectly_mutable_locals<'mir, 'tcx>(
item: &Item<'mir, 'tcx>,
) -> RefCell<IndirectlyMutableResults<'mir, 'tcx>> {
let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len());
let indirectly_mutable_locals = old_dataflow::do_dataflow(
item.tcx,
item.body,
item.def_id,
&item.tcx.get_attrs(item.def_id),
&dead_unwinds,
old_dataflow::IndirectlyMutableLocals::new(item.tcx, item.body, item.param_env),
|_, local| old_dataflow::DebugFormatted::new(&local),
);
let indirectly_mutable_locals = old_dataflow::DataflowResultsCursor::new(
indirectly_mutable_locals,
item.body,
);
RefCell::new(indirectly_mutable_locals)
}
impl Validator<'a, 'mir, 'tcx> {
pub fn new(
item: &'a Item<'mir, 'tcx>,
indirectly_mutable_locals: &'a RefCell<IndirectlyMutableResults<'mir, 'tcx>>,
) -> Self {
let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len());
let needs_drop = FlowSensitiveResolver::new(
let needs_drop = QualifCursor::new(
NeedsDrop,
item,
indirectly_mutable_locals,
&dead_unwinds,
);
let has_mut_interior = FlowSensitiveResolver::new(
let has_mut_interior = QualifCursor::new(
HasMutInterior,
item,
indirectly_mutable_locals,
&dead_unwinds,
);
let indirectly_mutable = old_dataflow::do_dataflow(
item.tcx,
item.body,
item.def_id,
&item.tcx.get_attrs(item.def_id),
&dead_unwinds,
old_dataflow::IndirectlyMutableLocals::new(item.tcx, item.body, item.param_env),
|_, local| old_dataflow::DebugFormatted::new(&local),
);
let indirectly_mutable = old_dataflow::DataflowResultsCursor::new(
indirectly_mutable,
item.body,
);
let qualifs = Qualifs {
needs_drop,
has_mut_interior,
indirectly_mutable,
};
Validator {
@ -122,14 +178,6 @@ impl Validator<'a, 'mir, 'tcx> {
}
}
/// Resets the `QualifResolver`s used by this `Validator` and returns them so they can be
/// reused.
pub fn into_qualifs(mut self) -> Qualifs<'a, 'mir, 'tcx> {
self.qualifs.needs_drop.reset();
self.qualifs.has_mut_interior.reset();
self.qualifs
}
pub fn take_errors(&mut self) -> Vec<(Span, String)> {
std::mem::replace(&mut self.errors, vec![])
}
@ -304,10 +352,16 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
// it depends on `HasMutInterior` being set for mutable borrows as well as values with
// interior mutability.
if let Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue {
let rvalue_has_mut_interior = {
let has_mut_interior = self.qualifs.has_mut_interior.get();
HasMutInterior::in_rvalue(&self.item, &|l| has_mut_interior.contains(l), rvalue)
};
// FIXME: Change the `in_*` methods to take a `FnMut` so we don't have to manually seek
// the cursors beforehand.
self.qualifs.has_mut_interior.cursor.seek_before(location);
self.qualifs.indirectly_mutable.seek(location);
let rvalue_has_mut_interior = HasMutInterior::in_rvalue(
&self.item,
&|local| self.qualifs.has_mut_interior_eager_seek(local),
rvalue,
);
if rvalue_has_mut_interior {
let is_derived_from_illegal_borrow = match borrowed_place.as_local() {
@ -402,9 +456,6 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
trace!("visit_statement: statement={:?} location={:?}", statement, location);
self.qualifs.needs_drop.visit_statement(statement, location);
self.qualifs.has_mut_interior.visit_statement(statement, location);
match statement.kind {
StatementKind::Assign(..) => {
self.super_statement(statement, location);
@ -424,15 +475,6 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
}
}
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
trace!("visit_terminator: terminator={:?} location={:?}", terminator, location);
self.qualifs.needs_drop.visit_terminator(terminator, location);
self.qualifs.has_mut_interior.visit_terminator(terminator, location);
self.super_terminator(terminator, location);
}
fn visit_terminator_kind(&mut self, kind: &TerminatorKind<'tcx>, location: Location) {
trace!("visit_terminator_kind: kind={:?} location={:?}", kind, location);
self.super_terminator_kind(kind, location);
@ -511,7 +553,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
let needs_drop = if let Some(local) = dropped_place.as_local() {
// Use the span where the local was declared as the span of the drop error.
err_span = self.body.local_decls[local].source_info.span;
self.qualifs.needs_drop.contains(local)
self.qualifs.needs_drop_lazy_seek(local, location)
} else {
true
};

View file

@ -968,8 +968,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
}
let item = new_checker::Item::new(self.tcx, self.def_id, self.body);
let mut_borrowed_locals = new_checker::validation::compute_indirectly_mutable_locals(&item);
let mut validator = new_checker::validation::Validator::new(&item, &mut_borrowed_locals);
let mut validator = new_checker::validation::Validator::new(&item);
validator.suppress_errors = !use_new_validator;
self.suppress_errors = use_new_validator;