Rollup merge of rust-lang#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, rust-lang#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 rust-lang#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

Author: Centril

src/librustc_mir/transform/check_consts/mod.rs

@@ -4,26 +4,27 @@
 //! 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;
 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()
 }

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> {

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,
                     }