Auto merge of #55720 - RalfJung:const-eval-raw, r=oli-obk

Make const_eval_raw query return just an AllocId

r? @oli-obk

Author: bors

Diff for: src/librustc/ich/impls_ty.rs

@@ -317,6 +317,10 @@ impl_stable_hash_for!(
         ByRef(id, alloc, offset),
     }
 );
+impl_stable_hash_for!(struct ::mir::interpret::RawConst<'tcx> {
+    alloc_id,
+    ty,
+});
 
 impl_stable_hash_for! {
     impl<Tag> for struct mir::interpret::Pointer<Tag> {

Diff for: src/librustc/mir/interpret/error.rs

@@ -16,7 +16,7 @@ use ty::{self, Ty, layout};
 use ty::layout::{Size, Align, LayoutError};
 use rustc_target::spec::abi::Abi;
 
-use super::{Pointer, InboundsCheck, ScalarMaybeUndef};
+use super::{RawConst, Pointer, InboundsCheck, ScalarMaybeUndef};
 
 use backtrace::Backtrace;
 
@@ -46,6 +46,7 @@ impl ErrorHandled {
     }
 }
 
+pub type ConstEvalRawResult<'tcx> = Result<RawConst<'tcx>, ErrorHandled>;
 pub type ConstEvalResult<'tcx> = Result<&'tcx ty::Const<'tcx>, ErrorHandled>;
 
 #[derive(Clone, Debug, RustcEncodable, RustcDecodable)]

Diff for: src/librustc/mir/interpret/mod.rs

@@ -22,10 +22,10 @@ mod pointer;
 
 pub use self::error::{
     EvalError, EvalResult, EvalErrorKind, AssertMessage, ConstEvalErr, struct_error,
-    FrameInfo, ConstEvalResult, ErrorHandled,
+    FrameInfo, ConstEvalRawResult, ConstEvalResult, ErrorHandled,
 };
 
-pub use self::value::{Scalar, ConstValue, ScalarMaybeUndef};
+pub use self::value::{Scalar, ScalarMaybeUndef, RawConst, ConstValue};
 
 pub use self::allocation::{
     InboundsCheck, Allocation, AllocationExtra,

Diff for: src/librustc/mir/interpret/value.rs

@@ -10,19 +10,28 @@
 
 use std::fmt;
 
-use ty::layout::{HasDataLayout, Size};
-use ty::subst::Substs;
-use hir::def_id::DefId;
+use crate::ty::{Ty, subst::Substs, layout::{HasDataLayout, Size}};
+use crate::hir::def_id::DefId;
 
 use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_extend, truncate};
 
+/// Represents the result of a raw const operation, pre-validation.
+#[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Hash)]
+pub struct RawConst<'tcx> {
+    // the value lives here, at offset 0, and that allocation definitely is a `AllocType::Memory`
+    // (so you can use `AllocMap::unwrap_memory`).
+    pub alloc_id: AllocId,
+    pub ty: Ty<'tcx>,
+}
+
 /// Represents a constant value in Rust. Scalar and ScalarPair are optimizations which
 /// matches the LocalValue optimizations for easy conversions between Value and ConstValue.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Hash)]
 pub enum ConstValue<'tcx> {
     /// Never returned from the `const_eval` query, but the HIR contains these frequently in order
     /// to allow HIR creation to happen for everything before needing to be able to run constant
     /// evaluation
+    /// FIXME: The query should then return a type that does not even have this variant.
     Unevaluated(DefId, &'tcx Substs<'tcx>),
 
     /// Used only for types with layout::abi::Scalar ABI and ZSTs

Diff for: src/librustc/ty/query/mod.rs

@@ -27,7 +27,7 @@ use middle::stability::{self, DeprecationEntry};
 use middle::lib_features::LibFeatures;
 use middle::lang_items::{LanguageItems, LangItem};
 use middle::exported_symbols::{SymbolExportLevel, ExportedSymbol};
-use mir::interpret::ConstEvalResult;
+use mir::interpret::{ConstEvalRawResult, ConstEvalResult};
 use mir::mono::CodegenUnit;
 use mir;
 use mir::interpret::GlobalId;
@@ -309,7 +309,7 @@ define_queries! { <'tcx>
         /// validation. Please add a comment to every use site explaining why using `const_eval`
         /// isn't sufficient
         [] fn const_eval_raw: const_eval_raw_dep_node(ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>)
-            -> ConstEvalResult<'tcx>,
+            -> ConstEvalRawResult<'tcx>,
 
         /// Results of evaluating const items or constants embedded in
         /// other items (such as enum variant explicit discriminants).

Diff for: src/librustc_mir/const_eval.rs

@@ -31,8 +31,8 @@ use rustc::util::common::ErrorReported;
 use syntax::ast::Mutability;
 use syntax::source_map::{Span, DUMMY_SP};
 
-use interpret::{self,
-    PlaceTy, MemPlace, OpTy, Operand, Immediate, Scalar, ConstValue, Pointer,
+use crate::interpret::{self,
+    PlaceTy, MPlaceTy, MemPlace, OpTy, Operand, Immediate, Scalar, RawConst, ConstValue, Pointer,
     EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup,
     Allocation, AllocId, MemoryKind,
     snapshot, RefTracking,
@@ -94,11 +94,12 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>(
     cid: GlobalId<'tcx>,
     mir: &'mir mir::Mir<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
-) -> EvalResult<'tcx, OpTy<'tcx>> {
+) -> EvalResult<'tcx, MPlaceTy<'tcx>> {
     let mut ecx = mk_borrowck_eval_cx(tcx, cid.instance, mir, DUMMY_SP).unwrap();
     eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env)
 }
 
+// FIXME: These two conversion functions are bad hacks.  We should just always use allocations.
 pub fn op_to_const<'tcx>(
     ecx: &CompileTimeEvalContext<'_, '_, 'tcx>,
     op: OpTy<'tcx>,
@@ -144,13 +145,20 @@ pub fn op_to_const<'tcx>(
     };
     Ok(ty::Const::from_const_value(ecx.tcx.tcx, val, op.layout.ty))
 }
+pub fn const_to_op<'tcx>(
+    ecx: &CompileTimeEvalContext<'_, '_, 'tcx>,
+    cnst: &ty::Const<'tcx>,
+) -> EvalResult<'tcx, OpTy<'tcx>> {
+    let op = ecx.const_value_to_op(cnst.val)?;
+    Ok(OpTy { op, layout: ecx.layout_of(cnst.ty)? })
+}
 
 fn eval_body_and_ecx<'a, 'mir, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     cid: GlobalId<'tcx>,
     mir: Option<&'mir mir::Mir<'tcx>>,
     param_env: ty::ParamEnv<'tcx>,
-) -> (EvalResult<'tcx, OpTy<'tcx>>, CompileTimeEvalContext<'a, 'mir, 'tcx>) {
+) -> (EvalResult<'tcx, MPlaceTy<'tcx>>, CompileTimeEvalContext<'a, 'mir, 'tcx>) {
     // we start out with the best span we have
     // and try improving it down the road when more information is available
     let span = tcx.def_span(cid.instance.def_id());
@@ -166,7 +174,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
     cid: GlobalId<'tcx>,
     mir: Option<&'mir mir::Mir<'tcx>>,
     param_env: ty::ParamEnv<'tcx>,
-) -> EvalResult<'tcx, OpTy<'tcx>> {
+) -> EvalResult<'tcx, MPlaceTy<'tcx>> {
     debug!("eval_body_using_ecx: {:?}, {:?}", cid, param_env);
     let tcx = ecx.tcx.tcx;
     let mut mir = match mir {
@@ -206,7 +214,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
     ecx.memory.intern_static(ret.ptr.to_ptr()?.alloc_id, mutability)?;
 
     debug!("eval_body_using_ecx done: {:?}", *ret);
-    Ok(ret.into())
+    Ok(ret)
 }
 
 impl<'tcx> Into<EvalError<'tcx>> for ConstEvalError {
@@ -494,7 +502,7 @@ pub fn const_field<'a, 'tcx>(
     let ecx = mk_eval_cx(tcx, instance, param_env).unwrap();
     let result = (|| {
         // get the operand again
-        let op = ecx.const_to_op(value)?;
+        let op = const_to_op(&ecx, value)?;
         // downcast
         let down = match variant {
             None => op,
@@ -521,7 +529,7 @@ pub fn const_variant_index<'a, 'tcx>(
 ) -> EvalResult<'tcx, VariantIdx> {
     trace!("const_variant_index: {:?}, {:?}", instance, val);
     let ecx = mk_eval_cx(tcx, instance, param_env).unwrap();
-    let op = ecx.const_to_op(val)?;
+    let op = const_to_op(&ecx, val)?;
     Ok(ecx.read_discriminant(op)?.1)
 }
 
@@ -534,15 +542,17 @@ pub fn error_to_const_error<'a, 'mir, 'tcx>(
     ConstEvalErr { error: error.kind, stacktrace, span: ecx.tcx.span }
 }
 
-fn validate_const<'a, 'tcx>(
+fn validate_and_turn_into_const<'a, 'tcx>(
     tcx: ty::TyCtxt<'a, 'tcx, 'tcx>,
-    constant: &'tcx ty::Const<'tcx>,
+    constant: RawConst<'tcx>,
     key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
 ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> {
     let cid = key.value;
     let ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap();
     let val = (|| {
-        let op = ecx.const_to_op(constant)?;
+        let op = ecx.raw_const_to_mplace(constant)?.into();
+        // FIXME: Once the visitor infrastructure landed, change validation to
+        // work directly on `MPlaceTy`.
         let mut ref_tracking = RefTracking::new(op);
         while let Some((op, path)) = ref_tracking.todo.pop() {
             ecx.validate_operand(
@@ -552,7 +562,10 @@ fn validate_const<'a, 'tcx>(
                 /* const_mode */ true,
             )?;
         }
-        Ok(constant)
+        // Now that we validated, turn this into a proper constant
+        let def_id = cid.instance.def.def_id();
+        let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none();
+        op_to_const(&ecx, op, normalize)
     })();
 
     val.map_err(|error| {
@@ -591,14 +604,14 @@ pub fn const_eval_provider<'a, 'tcx>(
         }
     }
     tcx.const_eval_raw(key).and_then(|val| {
-        validate_const(tcx, val, key)
+        validate_and_turn_into_const(tcx, val, key)
     })
 }
 
 pub fn const_eval_raw_provider<'a, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
-) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> {
+) -> ::rustc::mir::interpret::ConstEvalRawResult<'tcx> {
     // Because the constant is computed twice (once per value of `Reveal`), we are at risk of
     // reporting the same error twice here. To resolve this, we check whether we can evaluate the
     // constant in the more restrictive `Reveal::UserFacing`, which most likely already was
@@ -648,16 +661,11 @@ pub fn const_eval_raw_provider<'a, 'tcx>(
     };
 
     let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env);
-    res.and_then(|op| {
-        let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none();
-        if !normalize {
-            // Sanity check: These must always be a MemPlace
-            match op.op {
-                Operand::Indirect(_) => { /* all is good */ },
-                Operand::Immediate(_) => bug!("const eval gave us an Immediate"),
-            }
-        }
-        op_to_const(&ecx, op, normalize)
+    res.and_then(|place| {
+        Ok(RawConst {
+            alloc_id: place.to_ptr().expect("we allocated this ptr!").alloc_id,
+            ty: place.layout.ty
+        })
     }).map_err(|error| {
         let err = error_to_const_error(&ecx, error);
         // errors in statics are always emitted as fatal errors

Diff for: src/librustc_mir/interpret/eval_context.rs

@@ -588,18 +588,26 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         Ok(())
     }
 
-    pub fn const_eval(&self, gid: GlobalId<'tcx>) -> EvalResult<'tcx, &'tcx ty::Const<'tcx>> {
+    pub fn const_eval_raw(
+        &self,
+        gid: GlobalId<'tcx>,
+    ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
         let param_env = if self.tcx.is_static(gid.instance.def_id()).is_some() {
             ty::ParamEnv::reveal_all()
         } else {
             self.param_env
         };
-        self.tcx.const_eval(param_env.and(gid)).map_err(|err| {
+        // We use `const_eval_raw` here, and get an unvalidated result.  That is okay:
+        // Our result will later be validated anyway, and there seems no good reason
+        // to have to fail early here.  This is also more consistent with
+        // `Memory::get_static_alloc` which has to use `const_eval_raw` to avoid cycles.
+        let val = self.tcx.const_eval_raw(param_env.and(gid)).map_err(|err| {
             match err {
-                ErrorHandled::Reported => EvalErrorKind::ReferencedConstant.into(),
-                ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric.into(),
+                ErrorHandled::Reported => EvalErrorKind::ReferencedConstant,
+                ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric,
             }
-        })
+        })?;
+        self.raw_const_to_mplace(val)
     }
 
     pub fn dump_place(&self, place: Place<M::PointerTag>) {

Diff for: src/librustc_mir/interpret/memory.rs

@@ -28,7 +28,7 @@ use rustc_data_structures::fx::{FxHashSet, FxHashMap};
 use syntax::ast::Mutability;
 
 use super::{
-    Pointer, AllocId, Allocation, ConstValue, GlobalId, AllocationExtra, InboundsCheck,
+    Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InboundsCheck,
     EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic,
     Machine, AllocMap, MayLeak, ScalarMaybeUndef, ErrorHandled,
 };
@@ -374,14 +374,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
                 ErrorHandled::Reported => EvalErrorKind::ReferencedConstant.into(),
                 ErrorHandled::TooGeneric => EvalErrorKind::TooGeneric.into(),
             }
-        }).map(|const_val| {
-            if let ConstValue::ByRef(_, allocation, _) = const_val.val {
-                // We got tcx memory. Let the machine figure out whether and how to
-                // turn that into memory with the right pointer tag.
-                M::adjust_static_allocation(allocation)
-            } else {
-                bug!("Matching on non-ByRef static")
-            }
+        }).map(|raw_const| {
+            let allocation = tcx.alloc_map.lock().unwrap_memory(raw_const.alloc_id);
+            // We got tcx memory. Let the machine figure out whether and how to
+            // turn that into memory with the right pointer tag.
+            M::adjust_static_allocation(allocation)
         })
     }
 

Diff for: src/librustc_mir/interpret/operand.rs

@@ -13,7 +13,7 @@
 
 use std::convert::TryInto;
 
-use rustc::{mir, ty};
+use rustc::mir;
 use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx};
 
 use rustc::mir::interpret::{
@@ -535,19 +535,21 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             .collect()
     }
 
-    // Also used e.g. when miri runs into a constant.
-    pub(super) fn const_value_to_op(
+    // Used when miri runs into a constant, and by CTFE.
+    // FIXME: CTFE should use allocations, then we can make this private (embed it into
+    // `eval_operand`, ideally).
+    pub(crate) fn const_value_to_op(
         &self,
         val: ConstValue<'tcx>,
     ) -> EvalResult<'tcx, Operand<M::PointerTag>> {
         trace!("const_value_to_op: {:?}", val);
         match val {
             ConstValue::Unevaluated(def_id, substs) => {
                 let instance = self.resolve(def_id, substs)?;
-                self.global_to_op(GlobalId {
+                Ok(*OpTy::from(self.const_eval_raw(GlobalId {
                     instance,
                     promoted: None,
-                })
+                })?))
             }
             ConstValue::ByRef(id, alloc, offset) => {
                 // We rely on mutability being set correctly in that allocation to prevent writes
@@ -565,21 +567,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 Ok(Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag()),
         }
     }
-    pub fn const_to_op(
-        &self,
-        cnst: &ty::Const<'tcx>,
-    ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        let op = self.const_value_to_op(cnst.val)?;
-        Ok(OpTy { op, layout: self.layout_of(cnst.ty)? })
-    }
-
-    pub(super) fn global_to_op(
-        &self,
-        gid: GlobalId<'tcx>
-    ) -> EvalResult<'tcx, Operand<M::PointerTag>> {
-        let cv = self.const_eval(gid)?;
-        self.const_value_to_op(cv.val)
-    }
 
     /// Read discriminant, return the runtime value as well as the variant index.
     pub fn read_discriminant(