about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMazdak Farrokhzad <twingoow@gmail.com>2019-10-27 16:46:56 +0100
committerGitHub <noreply@github.com>2019-10-27 16:46:56 +0100
commitdae8ded9f5821f9dadde15ebca29543a1f20b072 (patch)
tree689d86610502f5fcf547ecb91455567c05251868
parent0982060a69e7a3faaa705d0e64bf651872b2f640 (diff)
parentb93cdbce36b1a42c77b5f3f721d7d64dec80f8f3 (diff)
downloadrust-dae8ded9f5821f9dadde15ebca29543a1f20b072.tar.gz
rust-dae8ded9f5821f9dadde15ebca29543a1f20b072.zip
Rollup merge of #65839 - ecstatic-morse:promo-sanity-fixes, r=eddyb
Clean up `check_consts` now that new promotion pass is implemented

`check_consts::resolver` contained a layer of abstraction (`QualifResolver`) to allow the existing, eager style of qualif propagation to work with either a dataflow results cursor or by applying the transfer function directly (if dataflow was not needed e.g. for promotion). However, #63812 uses a different, lazy paradigm for checking promotability, which makes this unnecessary. This PR cleans up `check_consts::validation` to use `FlowSensitiveResolver` directly, instead of through the now obselete `QualifResolver` API.

Also, this contains a few commits (the first four) that address some FIXMEs in #63812 regarding code duplication. They could be split out, but I think they will be relatively noncontroversial? Notably, `validation::Mode` is renamed to `ConstKind` and used in `promote_consts` to denote what kind of item we are in.

This is best reviewed commit-by-commit and is low priority.

r? @eddyb
-rw-r--r--src/librustc_mir/transform/check_consts/mod.rs109
-rw-r--r--src/librustc_mir/transform/check_consts/ops.rs33
-rw-r--r--src/librustc_mir/transform/check_consts/qualifs.rs21
-rw-r--r--src/librustc_mir/transform/check_consts/resolver.rs153
-rw-r--r--src/librustc_mir/transform/check_consts/validation.rs207
-rw-r--r--src/librustc_mir/transform/promote_consts.rs85
-rw-r--r--src/librustc_mir/transform/qualify_consts.rs3
7 files changed, 240 insertions, 371 deletions
diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs
index 0c643f46243..364e23ed8d0 100644
--- a/src/librustc_mir/transform/check_consts/mod.rs
+++ b/src/librustc_mir/transform/check_consts/mod.rs
@@ -4,10 +4,12 @@
 //! has interior mutability or needs to be dropped, as well as the visitor that emits errors when
 //! it finds operations that are invalid in a certain context.
 
-use rustc::hir::def_id::DefId;
+use rustc::hir::{self, def_id::DefId};
 use rustc::mir;
 use rustc::ty::{self, TyCtxt};
 
+use std::fmt;
+
 pub use self::qualifs::Qualif;
 
 pub mod ops;
@@ -15,15 +17,14 @@ pub mod qualifs;
 mod resolver;
 pub mod validation;
 
-/// Information about the item currently being validated, as well as a reference to the global
+/// Information about the item currently being const-checked, as well as a reference to the global
 /// context.
 pub struct Item<'mir, 'tcx> {
-    body: &'mir mir::Body<'tcx>,
-    tcx: TyCtxt<'tcx>,
-    def_id: DefId,
-    param_env: ty::ParamEnv<'tcx>,
-    mode: validation::Mode,
-    for_promotion: bool,
+    pub body: &'mir mir::Body<'tcx>,
+    pub tcx: TyCtxt<'tcx>,
+    pub def_id: DefId,
+    pub param_env: ty::ParamEnv<'tcx>,
+    pub const_kind: Option<ConstKind>,
 }
 
 impl Item<'mir, 'tcx> {
@@ -33,43 +34,91 @@ impl Item<'mir, 'tcx> {
         body: &'mir mir::Body<'tcx>,
     ) -> Self {
         let param_env = tcx.param_env(def_id);
-        let mode = validation::Mode::for_item(tcx, def_id)
-            .expect("const validation must only be run inside a const context");
+        let const_kind = ConstKind::for_item(tcx, def_id);
 
         Item {
             body,
             tcx,
             def_id,
             param_env,
-            mode,
-            for_promotion: false,
+            const_kind,
         }
     }
 
-    // HACK(eddyb) this is to get around the panic for a runtime fn from `Item::new`.
-    // Also, it allows promoting `&mut []`.
-    pub fn for_promotion(
-        tcx: TyCtxt<'tcx>,
-        def_id: DefId,
-        body: &'mir mir::Body<'tcx>,
-    ) -> Self {
-        let param_env = tcx.param_env(def_id);
-        let mode = validation::Mode::for_item(tcx, def_id)
-            .unwrap_or(validation::Mode::ConstFn);
+    /// Returns the kind of const context this `Item` represents (`const`, `static`, etc.).
+    ///
+    /// Panics if this `Item` is not const.
+    pub fn const_kind(&self) -> ConstKind {
+        self.const_kind.expect("`const_kind` must not be called on a non-const fn")
+    }
+}
 
-        Item {
-            body,
-            tcx,
-            def_id,
-            param_env,
-            mode,
-            for_promotion: true,
+/// The kinds of items which require compile-time evaluation.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum ConstKind {
+    /// A `static` item.
+    Static,
+    /// A `static mut` item.
+    StaticMut,
+    /// A `const fn` item.
+    ConstFn,
+    /// A `const` item or an anonymous constant (e.g. in array lengths).
+    Const,
+}
+
+impl ConstKind {
+    /// Returns the validation mode for the item with the given `DefId`, or `None` if this item
+    /// does not require validation (e.g. a non-const `fn`).
+    pub fn for_item(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<Self> {
+        use hir::BodyOwnerKind as HirKind;
+
+        let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
+
+        let mode = match tcx.hir().body_owner_kind(hir_id) {
+            HirKind::Closure => return None,
+
+            HirKind::Fn if tcx.is_const_fn(def_id) => ConstKind::ConstFn,
+            HirKind::Fn => return None,
+
+            HirKind::Const => ConstKind::Const,
+
+            HirKind::Static(hir::MutImmutable) => ConstKind::Static,
+            HirKind::Static(hir::MutMutable) => ConstKind::StaticMut,
+        };
+
+        Some(mode)
+    }
+
+    pub fn is_static(self) -> bool {
+        match self {
+            ConstKind::Static | ConstKind::StaticMut => true,
+            ConstKind::ConstFn | ConstKind::Const => false,
+        }
+    }
+
+    /// Returns `true` if the value returned by this item must be `Sync`.
+    ///
+    /// This returns false for `StaticMut` since all accesses to one are `unsafe` anyway.
+    pub fn requires_sync(self) -> bool {
+        match self {
+            ConstKind::Static => true,
+            ConstKind::ConstFn | ConstKind::Const |  ConstKind::StaticMut => false,
         }
     }
 }
 
+impl fmt::Display for ConstKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match *self {
+            ConstKind::Const => write!(f, "constant"),
+            ConstKind::Static | ConstKind::StaticMut => write!(f, "static"),
+            ConstKind::ConstFn => write!(f, "constant function"),
+        }
+    }
+}
 
-fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool {
+/// Returns `true` if this `DefId` points to one of the official `panic` lang items.
+pub fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool {
     Some(def_id) == tcx.lang_items().panic_fn() ||
     Some(def_id) == tcx.lang_items().begin_panic_fn()
 }
diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs
index f457b739949..4b374cff809 100644
--- a/src/librustc_mir/transform/check_consts/ops.rs
+++ b/src/librustc_mir/transform/check_consts/ops.rs
@@ -8,8 +8,7 @@ use syntax::feature_gate::{emit_feature_err, GateIssue};
 use syntax::symbol::sym;
 use syntax_pos::{Span, Symbol};
 
-use super::Item;
-use super::validation::Mode;
+use super::{ConstKind, Item};
 
 /// An operation that is not *always* allowed in a const context.
 pub trait NonConstOp: std::fmt::Debug {
@@ -36,7 +35,7 @@ pub trait NonConstOp: std::fmt::Debug {
             span,
             E0019,
             "{} contains unimplemented expression type",
-            item.mode
+            item.const_kind()
         );
         if item.tcx.sess.teach(&err.get_code().unwrap()) {
             err.note("A function call isn't allowed in the const's initialization expression \
@@ -76,7 +75,7 @@ impl NonConstOp for FnCallNonConst {
             E0015,
             "calls in {}s are limited to constant functions, \
              tuple structs and tuple variants",
-            item.mode,
+            item.const_kind(),
         );
         err.emit();
     }
@@ -121,8 +120,8 @@ impl NonConstOp for HeapAllocation {
 
     fn emit_error(&self, item: &Item<'_, '_>, span: Span) {
         let mut err = struct_span_err!(item.tcx.sess, span, E0010,
-                                       "allocations are not allowed in {}s", item.mode);
-        err.span_label(span, format!("allocation not allowed in {}s", item.mode));
+                                       "allocations are not allowed in {}s", item.const_kind());
+        err.span_label(span, format!("allocation not allowed in {}s", item.const_kind()));
         if item.tcx.sess.teach(&err.get_code().unwrap()) {
             err.note(
                 "The value of statics and constants must be known at compile time, \
@@ -146,7 +145,7 @@ impl NonConstOp for LiveDrop {
         struct_span_err!(item.tcx.sess, span, E0493,
                          "destructors cannot be evaluated at compile-time")
             .span_label(span, format!("{}s cannot evaluate destructors",
-                                      item.mode))
+                                      item.const_kind()))
             .emit();
     }
 }
@@ -163,9 +162,9 @@ impl NonConstOp for MutBorrow {
         if let BorrowKind::Mut { .. } = kind {
             let mut err = struct_span_err!(item.tcx.sess, span, E0017,
                                            "references in {}s may only refer \
-                                            to immutable values", item.mode);
+                                            to immutable values", item.const_kind());
             err.span_label(span, format!("{}s require immutable values",
-                                                item.mode));
+                                                item.const_kind()));
             if item.tcx.sess.teach(&err.get_code().unwrap()) {
                 err.note("References in statics and constants may only refer \
                           to immutable values.\n\n\
@@ -202,7 +201,7 @@ impl NonConstOp for Panic {
             sym::const_panic,
             span,
             GateIssue::Language,
-            &format!("panicking in {}s is unstable", item.mode),
+            &format!("panicking in {}s is unstable", item.const_kind()),
         );
     }
 }
@@ -220,7 +219,7 @@ impl NonConstOp for RawPtrComparison {
             sym::const_compare_raw_pointers,
             span,
             GateIssue::Language,
-            &format!("comparing raw pointers inside {}", item.mode),
+            &format!("comparing raw pointers inside {}", item.const_kind()),
         );
     }
 }
@@ -238,7 +237,7 @@ impl NonConstOp for RawPtrDeref {
             span, GateIssue::Language,
             &format!(
                 "dereferencing raw pointers in {}s is unstable",
-                item.mode,
+                item.const_kind(),
             ),
         );
     }
@@ -257,7 +256,7 @@ impl NonConstOp for RawPtrToIntCast {
             span, GateIssue::Language,
             &format!(
                 "casting pointers to integers in {}s is unstable",
-                item.mode,
+                item.const_kind(),
             ),
         );
     }
@@ -268,13 +267,13 @@ impl NonConstOp for RawPtrToIntCast {
 pub struct StaticAccess;
 impl NonConstOp for StaticAccess {
     fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool {
-        item.mode.is_static()
+        item.const_kind().is_static()
     }
 
     fn emit_error(&self, item: &Item<'_, '_>, span: Span) {
         let mut err = struct_span_err!(item.tcx.sess, span, E0013,
                                         "{}s cannot refer to statics, use \
-                                        a constant instead", item.mode);
+                                        a constant instead", item.const_kind());
         if item.tcx.sess.teach(&err.get_code().unwrap()) {
             err.note(
                 "Static and const variables can refer to other const variables. \
@@ -313,7 +312,7 @@ impl NonConstOp for Transmute {
             &item.tcx.sess.parse_sess, sym::const_transmute,
             span, GateIssue::Language,
             &format!("The use of std::mem::transmute() \
-            is gated in {}s", item.mode));
+            is gated in {}s", item.const_kind()));
     }
 }
 
@@ -322,7 +321,7 @@ pub struct UnionAccess;
 impl NonConstOp for UnionAccess {
     fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool {
         // Union accesses are stable in all contexts except `const fn`.
-        item.mode != Mode::ConstFn || Self::feature_gate(item.tcx).unwrap()
+        item.const_kind() != ConstKind::ConstFn || Self::feature_gate(item.tcx).unwrap()
     }
 
     fn feature_gate(tcx: TyCtxt<'_>) -> Option<bool> {
diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs
index e666dd9571f..840ad303016 100644
--- a/src/librustc_mir/transform/check_consts/qualifs.rs
+++ b/src/librustc_mir/transform/check_consts/qualifs.rs
@@ -5,8 +5,7 @@ use rustc::mir::interpret::ConstValue;
 use rustc::ty::{self, Ty};
 use syntax_pos::DUMMY_SP;
 
-use super::Item as ConstCx;
-use super::validation::Mode;
+use super::{ConstKind, Item as ConstCx};
 
 #[derive(Clone, Copy)]
 pub struct QualifSet(u8);
@@ -236,13 +235,17 @@ impl Qualif for HasMutInterior {
                     // mutably without consequences.
                     match ty.kind {
                         // Inside a `static mut`, &mut [...] is also allowed.
-                        ty::Array(..) | ty::Slice(_) if cx.mode == Mode::StaticMut => {},
-
-                        // FIXME(eddyb) the `cx.for_promotion` condition
-                        // seems unnecessary, given that this is merely a ZST.
-                        ty::Array(_, len)
-                            if len.try_eval_usize(cx.tcx, cx.param_env) == Some(0)
-                                && cx.for_promotion => {},
+                        | ty::Array(..)
+                        | ty::Slice(_)
+                        if cx.const_kind == Some(ConstKind::StaticMut)
+                        => {},
+
+                        // FIXME(eddyb): We only return false for `&mut []` outside a const
+                        // context which seems unnecessary given that this is merely a ZST.
+                        | ty::Array(_, len)
+                        if len.try_eval_usize(cx.tcx, cx.param_env) == Some(0)
+                            && cx.const_kind == None
+                        => {},
 
                         _ => return true,
                     }
diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs
index d3f1c760724..8909ef7db68 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<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>,
diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs
index fa746639736..244d434a51e 100644
--- a/src/librustc_mir/transform/check_consts/validation.rs
+++ b/src/librustc_mir/transform/check_consts/validation.rs
@@ -1,24 +1,23 @@
 //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations.
 
-use rustc::hir::{self, def_id::DefId};
 use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext};
 use rustc::mir::*;
 use rustc::ty::cast::CastTy;
-use rustc::ty::{self, TyCtxt};
+use rustc::ty;
 use rustc_index::bit_set::BitSet;
 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::{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 {
@@ -27,73 +26,75 @@ pub enum CheckOpResult {
     Allowed,
 }
 
-/// What kind of item we are in.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-pub enum Mode {
-    /// A `static` item.
-    Static,
-    /// A `static mut` item.
-    StaticMut,
-    /// A `const fn` item.
-    ConstFn,
-    /// A `const` item or an anonymous constant (e.g. in array lengths).
-    Const,
-}
-
-impl Mode {
-    /// Returns the validation mode for the item with the given `DefId`, or `None` if this item
-    /// does not require validation (e.g. a non-const `fn`).
-    pub fn for_item(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<Self> {
-        use hir::BodyOwnerKind as HirKind;
-
-        let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
+pub type IndirectlyMutableResults<'mir, 'tcx> =
+    old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>;
 
-        let mode = match tcx.hir().body_owner_kind(hir_id) {
-            HirKind::Closure => return None,
-
-            HirKind::Fn if tcx.is_const_fn(def_id) => Mode::ConstFn,
-            HirKind::Fn => return None,
-
-            HirKind::Const => Mode::Const,
+struct QualifCursor<'a, 'mir, 'tcx, Q: Qualif> {
+    cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>,
+    in_any_value_of_ty: BitSet<Local>,
+}
 
-            HirKind::Static(hir::MutImmutable) => Mode::Static,
-            HirKind::Static(hir::MutMutable) => Mode::StaticMut,
-        };
+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);
+            }
+        }
 
-        Some(mode)
+        QualifCursor {
+            cursor,
+            in_any_value_of_ty,
+        }
     }
+}
 
-    pub fn is_static(self) -> bool {
-        match self {
-            Mode::Static | Mode::StaticMut => true,
-            Mode::ConstFn | Mode::Const => false,
-        }
+pub struct Qualifs<'a, 'mir, 'tcx> {
+    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 the value returned by this item must be `Sync`.
+    /// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
     ///
-    /// This returns false for `StaticMut` since all accesses to one are `unsafe` anyway.
-    pub fn requires_sync(self) -> bool {
-        match self {
-            Mode::Static => true,
-            Mode::ConstFn | Mode::Const |  Mode::StaticMut => false,
+    /// 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)
     }
-}
 
-impl fmt::Display for Mode {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match *self {
-            Mode::Const => write!(f, "constant"),
-            Mode::Static | Mode::StaticMut => write!(f, "static"),
-            Mode::ConstFn => write!(f, "constant function"),
+    /// 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;
         }
-    }
-}
 
-pub struct Qualifs<'a, 'mir, 'tcx> {
-    has_mut_interior: FlowSensitiveResolver<'a, 'mir, 'tcx, HasMutInterior>,
-    needs_drop: FlowSensitiveResolver<'a, 'mir, 'tcx, NeedsDrop>,
+        self.has_mut_interior.cursor.get().contains(local)
+            || self.indirectly_mutable.get().contains(local)
+    }
 }
 
 pub struct Validator<'a, 'mir, 'tcx> {
@@ -128,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 {
@@ -187,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![])
     }
@@ -343,7 +326,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
                 let is_thread_local = self.tcx.has_attr(*def_id, sym::thread_local);
                 if is_thread_local {
                     self.check_op(ops::ThreadLocalAccess);
-                } else if self.mode == Mode::Static && context.is_mutating_use() {
+                } else if self.const_kind() == ConstKind::Static && context.is_mutating_use() {
                     // this is not strictly necessary as miri will also bail out
                     // For interior mutability we can't really catch this statically as that
                     // goes through raw pointers and intermediate temporaries, so miri has
@@ -369,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() {
@@ -467,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);
@@ -489,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);
@@ -576,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/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs
index 7aaff5735f6..3af08090853 100644
--- a/src/librustc_mir/transform/promote_consts.rs
+++ b/src/librustc_mir/transform/promote_consts.rs
@@ -12,7 +12,6 @@
 //! initialization and can otherwise silence errors, if
 //! move analysis runs after promotion on broken MIR.
 
-use rustc::hir;
 use rustc::hir::def_id::DefId;
 use rustc::mir::*;
 use rustc::mir::interpret::ConstValue;
@@ -30,7 +29,7 @@ use rustc_target::spec::abi::Abi;
 
 use std::{iter, mem, usize};
 
-use crate::transform::check_consts::{qualifs, Item as ConstCx};
+use crate::transform::check_consts::{qualifs, Item, ConstKind, is_lang_panic_fn};
 
 /// State of a temporary during collection and promotion.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -224,18 +223,13 @@ pub fn collect_temps_and_candidates(
     (collector.temps, collector.candidates)
 }
 
+/// Checks whether locals that appear in a promotion context (`Candidate`) are actually promotable.
+///
+/// This wraps an `Item`, and has access to all fields of that `Item` via `Deref` coercion.
 struct Validator<'a, 'tcx> {
-    tcx: TyCtxt<'tcx>,
-    param_env: ty::ParamEnv<'tcx>,
-    body: &'a Body<'tcx>,
-    is_static: bool,
-    is_static_mut: bool,
-    is_non_const_fn: bool,
+    item: Item<'a, 'tcx>,
     temps: &'a IndexVec<Local, TempState>,
 
-    // FIXME(eddyb) deduplicate the data in this vs other fields.
-    const_cx: ConstCx<'a, 'tcx>,
-
     /// Explicit promotion happens e.g. for constant arguments declared via
     /// `rustc_args_required_const`.
     /// Implicit promotion has almost the same rules, except that disallows `const fn`
@@ -245,14 +239,17 @@ struct Validator<'a, 'tcx> {
     explicit: bool,
 }
 
-struct Unpromotable;
+impl std::ops::Deref for Validator<'a, 'tcx> {
+    type Target = Item<'a, 'tcx>;
 
-impl<'tcx> Validator<'_, 'tcx> {
-    fn is_const_panic_fn(&self, def_id: DefId) -> bool {
-        Some(def_id) == self.tcx.lang_items().panic_fn() ||
-        Some(def_id) == self.tcx.lang_items().begin_panic_fn()
+    fn deref(&self) -> &Self::Target {
+        &self.item
     }
+}
+
+struct Unpromotable;
 
+impl<'tcx> Validator<'_, 'tcx> {
     fn validate_candidate(&self, candidate: Candidate) -> Result<(), Unpromotable> {
         match candidate {
             Candidate::Ref(loc) => {
@@ -317,13 +314,14 @@ impl<'tcx> Validator<'_, 'tcx> {
                         if self.qualif_local::<qualifs::NeedsDrop>(base) {
                             return Err(Unpromotable);
                         }
+
                         if let BorrowKind::Mut { .. } = kind {
                             let ty = place.ty(self.body, self.tcx).ty;
 
                             // In theory, any zero-sized value could be borrowed
                             // mutably without consequences. However, only &mut []
                             // is allowed right now, and only in functions.
-                            if self.is_static_mut {
+                            if self.const_kind == Some(ConstKind::StaticMut) {
                                 // Inside a `static mut`, &mut [...] is also allowed.
                                 match ty.kind {
                                     ty::Array(..) | ty::Slice(_) => {}
@@ -333,7 +331,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                                 // FIXME(eddyb) the `self.is_non_const_fn` condition
                                 // seems unnecessary, given that this is merely a ZST.
                                 match len.try_eval_usize(self.tcx, self.param_env) {
-                                    Some(0) if self.is_non_const_fn => {},
+                                    Some(0) if self.const_kind.is_none() => {},
                                     _ => return Err(Unpromotable),
                                 }
                             } else {
@@ -386,7 +384,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                 let statement = &self.body[loc.block].statements[loc.statement_index];
                 match &statement.kind {
                     StatementKind::Assign(box(_, rhs)) => {
-                        Q::in_rvalue(&self.const_cx, per_local, rhs)
+                        Q::in_rvalue(&self.item, per_local, rhs)
                     }
                     _ => {
                         span_bug!(statement.source_info.span, "{:?} is not an assignment",
@@ -398,7 +396,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                 match &terminator.kind {
                     TerminatorKind::Call { func, args, .. } => {
                         let return_ty = self.body.local_decls[local].ty;
-                        Q::in_call(&self.const_cx, per_local, func, args, return_ty)
+                        Q::in_call(&self.item, per_local, func, args, return_ty)
                     }
                     kind => {
                         span_bug!(terminator.source_info.span, "{:?} not promotable", kind);
@@ -462,8 +460,8 @@ impl<'tcx> Validator<'_, 'tcx> {
             } => {
                 // Only allow statics (not consts) to refer to other statics.
                 // FIXME(eddyb) does this matter at all for promotion?
-                let allowed = self.is_static || self.is_static_mut;
-                if !allowed {
+                let is_static = self.const_kind.map_or(false, |k| k.is_static());
+                if !is_static {
                     return Err(Unpromotable);
                 }
 
@@ -490,7 +488,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                     }
 
                     ProjectionElem::Field(..) => {
-                        if self.is_non_const_fn {
+                        if self.const_kind.is_none() {
                             let base_ty =
                                 Place::ty_from(place.base, proj_base, self.body, self.tcx).ty;
                             if let Some(def) = base_ty.ty_adt_def() {
@@ -545,7 +543,7 @@ impl<'tcx> Validator<'_, 'tcx> {
 
     fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
         match *rvalue {
-            Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) if self.is_non_const_fn => {
+            Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) if self.const_kind.is_none() => {
                 let operand_ty = operand.ty(self.body, self.tcx);
                 let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast");
                 let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast");
@@ -559,7 +557,7 @@ impl<'tcx> Validator<'_, 'tcx> {
                 }
             }
 
-            Rvalue::BinaryOp(op, ref lhs, _) if self.is_non_const_fn => {
+            Rvalue::BinaryOp(op, ref lhs, _) if self.const_kind.is_none() => {
                 if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind {
                     assert!(op == BinOp::Eq || op == BinOp::Ne ||
                             op == BinOp::Le || op == BinOp::Lt ||
@@ -600,17 +598,17 @@ impl<'tcx> Validator<'_, 'tcx> {
                     // In theory, any zero-sized value could be borrowed
                     // mutably without consequences. However, only &mut []
                     // is allowed right now, and only in functions.
-                    if self.is_static_mut {
+                    if self.const_kind == Some(ConstKind::StaticMut) {
                         // Inside a `static mut`, &mut [...] is also allowed.
                         match ty.kind {
                             ty::Array(..) | ty::Slice(_) => {}
                             _ => return Err(Unpromotable),
                         }
                     } else if let ty::Array(_, len) = ty.kind {
-                        // FIXME(eddyb) the `self.is_non_const_fn` condition
-                        // seems unnecessary, given that this is merely a ZST.
+                        // FIXME(eddyb): We only return `Unpromotable` for `&mut []` inside a
+                        // const context which seems unnecessary given that this is merely a ZST.
                         match len.try_eval_usize(self.tcx, self.param_env) {
-                            Some(0) if self.is_non_const_fn => {},
+                            Some(0) if self.const_kind.is_none() => {},
                             _ => return Err(Unpromotable),
                         }
                     } else {
@@ -683,7 +681,7 @@ impl<'tcx> Validator<'_, 'tcx> {
     ) -> Result<(), Unpromotable> {
         let fn_ty = callee.ty(self.body, self.tcx);
 
-        if !self.explicit && self.is_non_const_fn {
+        if !self.explicit && self.const_kind.is_none() {
             if let ty::FnDef(def_id, _) = fn_ty.kind {
                 // Never promote runtime `const fn` calls of
                 // functions without `#[rustc_promotable]`.
@@ -697,7 +695,7 @@ impl<'tcx> Validator<'_, 'tcx> {
             ty::FnDef(def_id, _) => {
                 self.tcx.is_const_fn(def_id) ||
                 self.tcx.is_unstable_const_fn(def_id).is_some() ||
-                self.is_const_panic_fn(def_id)
+                is_lang_panic_fn(self.tcx, self.def_id)
             }
             _ => false,
         };
@@ -714,6 +712,7 @@ impl<'tcx> Validator<'_, 'tcx> {
     }
 }
 
+// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`.
 pub fn validate_candidates(
     tcx: TyCtxt<'tcx>,
     body: &Body<'tcx>,
@@ -722,33 +721,11 @@ pub fn validate_candidates(
     candidates: &[Candidate],
 ) -> Vec<Candidate> {
     let mut validator = Validator {
-        tcx,
-        param_env: tcx.param_env(def_id),
-        body,
-        is_static: false,
-        is_static_mut: false,
-        is_non_const_fn: false,
+        item: Item::new(tcx, def_id, body),
         temps,
-
-        const_cx: ConstCx::for_promotion(tcx, def_id, body),
-
         explicit: false,
     };
 
-    // FIXME(eddyb) remove the distinctions that make this necessary.
-    let id = tcx.hir().as_local_hir_id(def_id).unwrap();
-    match tcx.hir().body_owner_kind(id) {
-        hir::BodyOwnerKind::Closure => validator.is_non_const_fn = true,
-        hir::BodyOwnerKind::Fn => {
-            if !tcx.is_const_fn(def_id) {
-                validator.is_non_const_fn = true;
-            }
-        },
-        hir::BodyOwnerKind::Static(hir::MutImmutable) => validator.is_static = true,
-        hir::BodyOwnerKind::Static(hir::MutMutable) => validator.is_static_mut = true,
-        _ => {}
-    }
-
     candidates.iter().copied().filter(|&candidate| {
         validator.explicit = match candidate {
             Candidate::Ref(_) |
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index 21feeb1fad6..2f77cd5ddf7 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;