Make const_eval_raw query return just an AllocId

Author: RalfJung

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

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)]

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,

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

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).

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,13 @@ 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: This thing is a bad hack. We should get rid of it.  Ideally constants are always
+// in an allocation.
 pub fn op_to_const<'tcx>(
     ecx: &CompileTimeEvalContext<'_, '_, 'tcx>,
     op: OpTy<'tcx>,
@@ -150,7 +152,7 @@ fn eval_body_and_ecx<'a, 'mir, '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 +168,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 +208,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 {
@@ -534,15 +536,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 +556,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 +598,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 +655,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

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

src/librustc_mir/interpret/operand.rs

@@ -536,7 +536,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
     }
 
     // Also used e.g. when miri runs into a constant.
-    pub(super) fn const_value_to_op(
+    // FIXME: Can we avoid converting with ConstValue and Const?  We should be using RawConst.
+    fn const_value_to_op(
         &self,
         val: ConstValue<'tcx>,
     ) -> EvalResult<'tcx, Operand<M::PointerTag>> {

src/librustc_mir/interpret/place.rs

@@ -20,12 +20,10 @@ use rustc::mir;
 use rustc::ty::{self, Ty};
 use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx};
 
-use rustc::mir::interpret::{
-    GlobalId, AllocId, Allocation, Scalar, EvalResult, Pointer, PointerArithmetic
-};
 use super::{
+    GlobalId, AllocId, Allocation, Scalar, EvalResult, Pointer, PointerArithmetic,
     EvalContext, Machine, AllocMap, AllocationExtra,
-    Immediate, ImmTy, ScalarMaybeUndef, Operand, OpTy, MemoryKind
+    RawConst, Immediate, ImmTy, ScalarMaybeUndef, Operand, OpTy, MemoryKind
 };
 
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
@@ -981,6 +979,19 @@ where
         Ok(OpTy { op, layout: place.layout })
     }
 
+    pub fn raw_const_to_mplace(
+        &self,
+        raw: RawConst<'tcx>,
+    ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
+        // This must be an allocation in `tcx`
+        assert!(self.tcx.alloc_map.lock().get(raw.alloc_id).is_some());
+        let layout = self.layout_of(raw.ty)?;
+        Ok(MPlaceTy::from_aligned_ptr(
+            Pointer::new(raw.alloc_id, Size::ZERO).with_default_tag(),
+            layout,
+        ))
+    }
+
     /// Turn a place with a `dyn Trait` type into a place with the actual dynamic type.
     /// Also return some more information so drop doesn't have to run the same code twice.
     pub(super) fn unpack_dyn_trait(&self, mplace: MPlaceTy<'tcx, M::PointerTag>)

src/librustc_mir/transform/const_prop.rs

@@ -309,7 +309,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                     eval_promoted(this.tcx, cid, this.mir, this.param_env)
                 })?;
                 trace!("evaluated promoted {:?} to {:?}", promoted, res);
-                Some((res, source_info.span))
+                Some((res.into(), source_info.span))
             },
             _ => None,
         }

src/test/ui/consts/const-err4.rs

@@ -16,7 +16,7 @@ union Foo {
 
 enum Bar {
     Boo = [unsafe { Foo { b: () }.a }; 4][3],
-    //~^ ERROR evaluation of constant value failed
+    //~^ ERROR it is undefined behavior to use this value
 }
 
 fn main() {

src/test/ui/consts/const-err4.stderr

@@ -1,8 +1,10 @@
-error[E0080]: evaluation of constant value failed
+error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-err4.rs:18:11
    |
 LL |     Boo = [unsafe { Foo { b: () }.a }; 4][3],
-   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error: aborting due to previous error
 

src/test/ui/consts/const-eval/const-pointer-values-in-various-types.rs

@@ -37,7 +37,7 @@ fn main() {
     //~^ ERROR it is undefined behavior to use this value
 
     const I32_REF_U128_UNION: u128 = unsafe { Nonsense { int_32_ref: &3 }.uint_128 };
-    //~^ ERROR any use of this value will cause an error
+    //~^ ERROR it is undefined behavior to use this value
 
     const I32_REF_I8_UNION: i8 = unsafe { Nonsense { int_32_ref: &3 }.int_8 };
     //~^ ERROR any use of this value will cause an error
@@ -52,7 +52,7 @@ fn main() {
     //~^ ERROR it is undefined behavior to use this value
 
     const I32_REF_I128_UNION: i128 = unsafe { Nonsense { int_32_ref: &3 }.int_128 };
-    //~^ ERROR any use of this value will cause an error
+    //~^ ERROR it is undefined behavior to use this value
 
     const I32_REF_F32_UNION: f32 = unsafe { Nonsense { int_32_ref: &3 }.float_32 };
     //~^ ERROR any use of this value will cause an error

src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr

@@ -40,11 +40,13 @@ LL |     const I32_REF_U64_UNION: u64 = unsafe { Nonsense { int_32_ref: &3 }.uin
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
-error: any use of this value will cause an error
+error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:39:5
    |
 LL |     const I32_REF_U128_UNION: u128 = unsafe { Nonsense { int_32_ref: &3 }.uint_128 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error: any use of this value will cause an error
   --> $DIR/const-pointer-values-in-various-types.rs:42:5
@@ -78,11 +80,13 @@ LL |     const I32_REF_I64_UNION: i64 = unsafe { Nonsense { int_32_ref: &3 }.int
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
-error: any use of this value will cause an error
+error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:54:5
    |
 LL |     const I32_REF_I128_UNION: i128 = unsafe { Nonsense { int_32_ref: &3 }.int_128 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error: any use of this value will cause an error
   --> $DIR/const-pointer-values-in-various-types.rs:57:5

src/test/ui/consts/const-eval/union-const-eval-field.rs

@@ -34,7 +34,8 @@ const fn read_field2() -> Field2 {
 }
 
 const fn read_field3() -> Field3 {
-    const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR any use of this value
+    const FIELD3: Field3 = unsafe { UNION.field3 };
+    //~^ ERROR it is undefined behavior to use this value
     FIELD3
 }
 

src/test/ui/consts/const-eval/union-const-eval-field.stderr

@@ -1,10 +1,11 @@
-error: any use of this value will cause an error
+error[E0080]: it is undefined behavior to use this value
   --> $DIR/union-const-eval-field.rs:37:5
    |
-LL |     const FIELD3: Field3 = unsafe { UNION.field3 }; //~ ERROR any use of this value
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes
+LL |     const FIELD3: Field3 = unsafe { UNION.field3 };
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized plain bits
    |
-   = note: #[deny(const_err)] on by default
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error: aborting due to previous error
 
+For more information about this error, try `rustc --explain E0080`.