diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs index d3f1c760724f1..8909ef7db683d 100644 --- a/src/librustc_mir/transform/check_consts/resolver.rs +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -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 { - /// 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; - - /// 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>, - cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>, - qualifs_per_local: BitSet, - - /// The value of `Q::in_any_value_of_ty` for each local. - qualifs_in_any_value_of_ty: BitSet, -} - -impl FlowSensitiveResolver<'a, 'mir, 'tcx, Q> -where - Q: Qualif, -{ - pub fn new( - _: Q, - item: &'a Item<'mir, 'tcx>, - indirectly_mutable_locals: &'a RefCell>, - dead_unwinds: &BitSet, - ) -> 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 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 QualifResolver for FlowSensitiveResolver<'_, '_, '_, Q> -where - Q: Qualif -{ - fn get(&mut self) -> &BitSet { - 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, diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index c145fb41c9695..244d434a51eab 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -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, +} + +impl QualifCursor<'a, 'mir, 'tcx, Q> { + pub fn new( + q: Q, + item: &'a Item<'mir, 'tcx>, + dead_unwinds: &BitSet, + ) -> 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> { - 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>, ) -> 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 }; diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 21feeb1fad61d..2f77cd5ddf716 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -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;