Auto merge of #63812 - eddyb:promo-sanity, r=oli-obk

rustc_mir: double-check const-promotion candidates for sanity.

Previously, const promotion involved tracking information about the value in a MIR local (or any part of the computation leading up to that value), aka "qualifs", in a quite stateful manner, which is hard to extend to arbitrary CFGs without a dataflow pass.

However, the nature of the promotion we do is that it's effectively an SSA-like "tree" (or DAG, really), of assigned-once locals - which is how we can take them from the original MIR in the first place.
This structure means that the subset of the MIR responsible for computing any given part of a const-promoted value is readily analyzable by walking that tree/DAG.

This PR implements such an analysis in `promote_consts`, reusing the `HasMutInterior` / `NeedsDrop` computation from `qualify_consts`, but reimplementing the equivalent of `IsNotPromotable` / `IsNotImplicitlyPromotable`.

Eventually we should be able to remove `IsNotPromotable` / `IsNotImplicitlyPromotable` from `qualify_consts`, which will simplify @ecstatic-morse's dataflow-based const-checking efforts.

But currently this is mainly for a crater check-only run - it will compare the results from the old promotion collection and the new promotion validation and ICE if they don't match.

r? @oli-obk

Author: bors

src/librustc_mir/transform/check_consts/mod.rs

@@ -11,7 +11,7 @@ use rustc::ty::{self, TyCtxt};
 pub use self::qualifs::Qualif;
 
 pub mod ops;
-mod qualifs;
+pub mod qualifs;
 mod resolver;
 pub mod validation;
 
@@ -23,6 +23,7 @@ pub struct Item<'mir, 'tcx> {
     def_id: DefId,
     param_env: ty::ParamEnv<'tcx>,
     mode: validation::Mode,
+    for_promotion: bool,
 }
 
 impl Item<'mir, 'tcx> {
@@ -41,6 +42,28 @@ impl Item<'mir, 'tcx> {
             def_id,
             param_env,
             mode,
+            for_promotion: false,
+        }
+    }
+
+    // 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);
+
+        Item {
+            body,
+            tcx,
+            def_id,
+            param_env,
+            mode,
+            for_promotion: true,
         }
     }
 }

src/librustc_mir/transform/check_consts/qualifs.rs

@@ -3,7 +3,6 @@
 use rustc::mir::*;
 use rustc::mir::interpret::ConstValue;
 use rustc::ty::{self, Ty};
-use rustc_index::bit_set::BitSet;
 use syntax_pos::DUMMY_SP;
 
 use super::Item as ConstCx;
@@ -44,7 +43,7 @@ pub trait Qualif {
 
     fn in_projection_structurally(
         cx: &ConstCx<'_, 'tcx>,
-        per_local: &BitSet<Local>,
+        per_local: &impl Fn(Local) -> bool,
         place: PlaceRef<'_, 'tcx>,
     ) -> bool {
         if let [proj_base @ .., elem] = place.projection {
@@ -65,7 +64,7 @@ pub trait Qualif {
                 ProjectionElem::ConstantIndex { .. } |
                 ProjectionElem::Downcast(..) => qualif,
 
-                ProjectionElem::Index(local) => qualif || per_local.contains(*local),
+                ProjectionElem::Index(local) => qualif || per_local(*local),
             }
         } else {
             bug!("This should be called if projection is not empty");
@@ -74,22 +73,22 @@ pub trait Qualif {
 
     fn in_projection(
         cx: &ConstCx<'_, 'tcx>,
-        per_local: &BitSet<Local>,
+        per_local: &impl Fn(Local) -> bool,
         place: PlaceRef<'_, 'tcx>,
     ) -> bool {
         Self::in_projection_structurally(cx, per_local, place)
     }
 
     fn in_place(
         cx: &ConstCx<'_, 'tcx>,
-        per_local: &BitSet<Local>,
+        per_local: &impl Fn(Local) -> bool,
         place: PlaceRef<'_, 'tcx>,
     ) -> bool {
         match place {
             PlaceRef {
                 base: PlaceBase::Local(local),
                 projection: [],
-            } => per_local.contains(*local),
+            } => per_local(*local),
             PlaceRef {
                 base: PlaceBase::Static(box Static {
                     kind: StaticKind::Promoted(..),
@@ -112,7 +111,7 @@ pub trait Qualif {
 
     fn in_operand(
         cx: &ConstCx<'_, 'tcx>,
-        per_local: &BitSet<Local>,
+        per_local: &impl Fn(Local) -> bool,
         operand: &Operand<'tcx>,
     ) -> bool {
         match *operand {
@@ -143,7 +142,7 @@ pub trait Qualif {
 
     fn in_rvalue_structurally(
         cx: &ConstCx<'_, 'tcx>,
-        per_local: &BitSet<Local>,
+        per_local: &impl Fn(Local) -> bool,
         rvalue: &Rvalue<'tcx>,
     ) -> bool {
         match *rvalue {
@@ -185,13 +184,17 @@ pub trait Qualif {
         }
     }
 
-    fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet<Local>, rvalue: &Rvalue<'tcx>) -> bool {
+    fn in_rvalue(
+        cx: &ConstCx<'_, 'tcx>,
+        per_local: &impl Fn(Local) -> bool,
+        rvalue: &Rvalue<'tcx>,
+    ) -> bool {
         Self::in_rvalue_structurally(cx, per_local, rvalue)
     }
 
     fn in_call(
         cx: &ConstCx<'_, 'tcx>,
-        _per_local: &BitSet<Local>,
+        _per_local: &impl Fn(Local) -> bool,
         _callee: &Operand<'tcx>,
         _args: &[Operand<'tcx>],
         return_ty: Ty<'tcx>,
@@ -216,7 +219,11 @@ impl Qualif for HasMutInterior {
         !ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)
     }
 
-    fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet<Local>, rvalue: &Rvalue<'tcx>) -> bool {
+    fn in_rvalue(
+        cx: &ConstCx<'_, 'tcx>,
+        per_local: &impl Fn(Local) -> bool,
+        rvalue: &Rvalue<'tcx>,
+    ) -> bool {
         match *rvalue {
             // Returning `true` for `Rvalue::Ref` indicates the borrow isn't
             // allowed in constants (and the `Checker` will error), and/or it
@@ -231,12 +238,11 @@ impl Qualif for HasMutInterior {
                         // Inside a `static mut`, &mut [...] is also allowed.
                         ty::Array(..) | ty::Slice(_) if cx.mode == Mode::StaticMut => {},
 
-                        // FIXME(ecstaticmorse): uncomment the following match arm to stop marking
-                        // `&mut []` as `HasMutInterior`.
-                        /*
-                        ty::Array(_, len) if len.try_eval_usize(cx.tcx, cx.param_env) == Some(0)
-                            => {},
-                        */
+                        // 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 => {},
 
                         _ => return true,
                     }
@@ -275,7 +281,11 @@ impl Qualif for NeedsDrop {
         ty.needs_drop(cx.tcx, cx.param_env)
     }
 
-    fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet<Local>, rvalue: &Rvalue<'tcx>) -> bool {
+    fn in_rvalue(
+        cx: &ConstCx<'_, 'tcx>,
+        per_local: &impl Fn(Local) -> bool,
+        rvalue: &Rvalue<'tcx>,
+    ) -> bool {
         if let Rvalue::Aggregate(ref kind, _) = *rvalue {
             if let AggregateKind::Adt(def, ..) = **kind {
                 if def.has_dtor(cx.tcx) {

src/librustc_mir/transform/check_consts/resolver.rs

@@ -81,7 +81,13 @@ where
         return_place: &mir::Place<'tcx>,
     ) {
         let return_ty = return_place.ty(self.item.body, self.item.tcx).ty;
-        let qualif = Q::in_call(self.item, &mut self.qualifs_per_local, func, args, return_ty);
+        let qualif = Q::in_call(
+            self.item,
+            &|l| self.qualifs_per_local.contains(l),
+            func,
+            args,
+            return_ty,
+        );
         if !return_place.is_indirect() {
             self.assign_qualif_direct(return_place, qualif);
         }
@@ -114,7 +120,7 @@ where
         rvalue: &mir::Rvalue<'tcx>,
         location: Location,
     ) {
-        let qualif = Q::in_rvalue(self.item, self.qualifs_per_local, rvalue);
+        let qualif = Q::in_rvalue(self.item, &|l| self.qualifs_per_local.contains(l), rvalue);
         if !place.is_indirect() {
             self.assign_qualif_direct(place, qualif);
         }
@@ -129,7 +135,7 @@ where
         // here; that occurs in `apply_call_return_effect`.
 
         if let mir::TerminatorKind::DropAndReplace { value, location: dest, .. } = kind {
-            let qualif = Q::in_operand(self.item, self.qualifs_per_local, value);
+            let qualif = Q::in_operand(self.item, &|l| self.qualifs_per_local.contains(l), value);
             if !dest.is_indirect() {
                 self.assign_qualif_direct(dest, qualif);
             }

src/librustc_mir/transform/check_consts/validation.rs

@@ -369,11 +369,10 @@ 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 = HasMutInterior::in_rvalue(
-                &self.item,
-                self.qualifs.has_mut_interior.get(),
-                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)
+            };
 
             if rvalue_has_mut_interior {
                 let is_derived_from_illegal_borrow = match borrowed_place.as_local() {