Stop using the const_eval query for initializers of statics

As a side effect, we now represent most promoteds as `ConstValue::Scalar` again. This is useful because all implict promoteds are just references anyway and most explicit promoteds are numeric arguments to `asm!` or SIMD instructions.
rust-lang · Dylan-DPC-zz · Aug 9, 2020 · Sep 9, 2020 · Sep 9, 2020 · Sep 10, 2020
commit 2d7ac728e4ce2aa1a77068e1f668be71d10116a0
diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs
@@ -12,7  12,7 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::Node;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
 use rustc_middle::mir::interpret::{
-    read_target_uint, Allocation, ConstValue, ErrorHandled, GlobalAlloc, Pointer,
     read_target_uint, Allocation, ErrorHandled, GlobalAlloc, Pointer,
 };
 use rustc_middle::mir::mono::MonoItem;
 use rustc_middle::ty::{self, Instance, Ty};
@@ -85,10  85,7 @@ pub fn codegen_static_initializer(
     cx: &CodegenCx<'ll, 'tcx>,
     def_id: DefId,
 ) -> Result<(&'ll Value, &'tcx Allocation), ErrorHandled> {
-    let alloc = match cx.tcx.const_eval_poly(def_id)? {
-        ConstValue::ByRef { alloc, offset } if offset.bytes() == 0 => alloc,
-        val => bug!("static const eval returned {:#?}", val),
-    };
     let alloc = cx.tcx.eval_static_initializer(def_id)?;
     Ok((const_alloc_to_llvm(cx, alloc), alloc))
 }
 

diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -13,7  13,7 @@ use rustc_ast as ast;
 use rustc_hir::lang_items::LangItem;
 use rustc_index::vec::Idx;
 use rustc_middle::mir;
-use rustc_middle::mir::interpret::{AllocId, ConstValue, Pointer, Scalar};
 use rustc_middle::mir::interpret::ConstValue;
 use rustc_middle::mir::AssertKind;
 use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt};
 use rustc_middle::ty::print::with_no_trimmed_paths;
@@ -867,24  867,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         let ty = constant.literal.ty;
                         let size = bx.layout_of(ty).size;
                         let scalar = match const_value {
-                            // Promoted constants are evaluated into a ByRef instead of a Scalar,
-                            // but we want the scalar value here.
-                            ConstValue::ByRef { alloc, offset } => {
-                                let ptr = Pointer::new(AllocId(0), offset);
-                                alloc
-                                    .read_scalar(&bx, ptr, size)
-                                    .and_then(|s| s.check_init())
-                                    .unwrap_or_else(|e| {
-                                        bx.tcx().sess.span_err(
-                                            span,
-                                            &format!("Could not evaluate asm const: {}", e),
-                                        );
-
-                                        // We are erroring out, just emit a dummy constant.
-                                        Scalar::from_u64(0)
-                                    })
-                            }
-                            _ => span_bug!(span, "expected ByRef for promoted asm const"),
                             ConstValue::Scalar(s) => s,
                             _ => span_bug!(
                                 span,
                                 "expected Scalar for promoted asm const, but got {:#?}",
                                 const_value
                             ),
                         };
                         let value = scalar.assert_bits(size);
                         let string = match ty.kind() {

diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -1473,21  1473,18 @@ declare_lint_pass!(
     UnusedBrokenConst => []
 );
 
-fn check_const(cx: &LateContext<'_>, body_id: hir::BodyId) {
-    let def_id = cx.tcx.hir().body_owner_def_id(body_id).to_def_id();
-    // trigger the query once for all constants since that will already report the errors
-    // FIXME: Use ensure here
-    let _ = cx.tcx.const_eval_poly(def_id);
-}
-
 impl<'tcx> LateLintPass<'tcx> for UnusedBrokenConst {
     fn check_item(&mut self, cx: &LateContext<'_>, it: &hir::Item<'_>) {
         match it.kind {
             hir::ItemKind::Const(_, body_id) => {
-                check_const(cx, body_id);
                 let def_id = cx.tcx.hir().body_owner_def_id(body_id).to_def_id();
                 // trigger the query once for all constants since that will already report the errors
                 // FIXME: Use ensure here
                 let _ = cx.tcx.const_eval_poly(def_id);
             }
             hir::ItemKind::Static(_, _, body_id) => {
-                check_const(cx, body_id);
                 let def_id = cx.tcx.hir().body_owner_def_id(body_id).to_def_id();
                 let _ = cx.tcx.eval_static_initializer(def_id);
             }
             _ => {}
         }

diff --git a/compiler/rustc_mir/src/const_eval/eval_queries.rs b/compiler/rustc_mir/src/const_eval/eval_queries.rs
@@ -182,7  182,7 @@ pub(super) fn op_to_const<'tcx>(
     }
 }
 
-fn validate_and_turn_into_const<'tcx>(
 fn turn_into_const<'tcx>(
     tcx: TyCtxt<'tcx>,
     constant: RawConst<'tcx>,
     key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
@@ -191,30  191,21 @@ fn validate_and_turn_into_const<'tcx>(
     let def_id = cid.instance.def.def_id();
     let is_static = tcx.is_static(def_id);
     let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env, is_static);
-    let val = (|| {
-        let mplace = ecx.raw_const_to_mplace(constant)?;
-        // Turn this into a proper constant.
-        // Statics/promoteds are always `ByRef`, for the rest `op_to_const` decides
-        // whether they become immediates.
-        if is_static || cid.promoted.is_some() {
-            let ptr = mplace.ptr.assert_ptr();
-            Ok(ConstValue::ByRef {
-                alloc: ecx.tcx.global_alloc(ptr.alloc_id).unwrap_memory(),
-                offset: ptr.offset,
-            })
-        } else {
-            Ok(op_to_const(&ecx, mplace.into()))
-        }
-    })();
 
-    // FIXME: Can this ever be an error and not be a compiler bug or can we just ICE here?
-    val.map_err(|error| {
     let mplace = ecx.raw_const_to_mplace(constant).map_err(|error| {
         // FIXME: Can the above ever error and not be a compiler bug or can we just ICE here?
         let err = ConstEvalErr::new(&ecx, error, None);
         err.struct_error(ecx.tcx, "it is undefined behavior to use this value", |mut diag| {
             diag.note(note_on_undefined_behavior_error());
             diag.emit();
         })
-    })
     })?;
     assert!(
         !is_static || cid.promoted.is_some(),
         "the const eval query should not be used for statics, use `const_eval_raw` instead"
     );
     // Turn this into a proper constant.
     Ok(op_to_const(&ecx, mplace.into()))
 }
 
 pub fn const_eval_validated_provider<'tcx>(
@@ -248,7  239,7 @@ pub fn const_eval_validated_provider<'tcx>(
         });
     }
 
-    tcx.const_eval_raw(key).and_then(|val| validate_and_turn_into_const(tcx, val, key))
     tcx.const_eval_raw(key).and_then(|val| turn_into_const(tcx, val, key))
 }
 
 pub fn const_eval_raw_provider<'tcx>(

diff --git a/compiler/rustc_mir/src/interpret/eval_context.rs b/compiler/rustc_mir/src/interpret/eval_context.rs
@@ -914,13  914,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         } else {
             self.param_env
         };
-        // 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.
-        // FIXME: We can hit delay_span_bug if this is an invalid const, interning finds
-        // that problem, but we never run validation to show an error. Can we ensure
-        // this does not happen?
         let val = self.tcx.const_eval_raw(param_env.and(gid))?;
         self.raw_const_to_mplace(val)
     }

diff --git a/compiler/rustc_mir/src/interpret/operand.rs b/compiler/rustc_mir/src/interpret/operand.rs
@@ -554,11  554,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             ty::ConstKind::Unevaluated(def, substs, promoted) => {
                 let instance = self.resolve(def.did, substs)?;
                 // We use `const_eval` here and `const_eval_raw` elsewhere in mir interpretation.
-                // The reason we use `const_eval_raw` everywhere else is to prevent cycles during
-                // validation, because validation automatically reads through any references, thus
-                // potentially requiring the current static to be evaluated again. This is not a
-                // problem here, because we are building an operand which means an actual read is
-                // happening.
                 // The reason we use `const_eval` here is that there can never be a `ty::ConstKind`
                 // that directly mentions the initializer of a static. Statics are always encoded
                 // as constants with vaule `&STATIC`.
                 return Ok(self.const_eval(GlobalId { instance, promoted }, val.ty)?);
             }
             ty::ConstKind::Infer(..)

diff --git a/compiler/rustc_mir/src/monomorphize/collector.rs b/compiler/rustc_mir/src/monomorphize/collector.rs
@@ -364,8  364,10 @@ fn collect_items_rec<'tcx>(
 
             recursion_depth_reset = None;
 
-            if let Ok(val) = tcx.const_eval_poly(def_id) {
-                collect_const_value(tcx, val, &mut neighbors);
             if let Ok(alloc) = tcx.eval_static_initializer(def_id) {
                 for &((), id) in alloc.relocations().values() {
                     collect_miri(tcx, id, &mut neighbors);
                 }
             }
         }
         MonoItem::Fn(instance) => {

diff --git a/compiler/rustc_mir/src/util/pretty.rs b/compiler/rustc_mir/src/util/pretty.rs
@@ -631,14  631,11 @@ pub fn write_allocations<'tcx>(
             None => write!(w, " (deallocated)")?,
             Some(GlobalAlloc::Function(inst)) => write!(w, " (fn: {})", inst)?,
             Some(GlobalAlloc::Static(did)) if !tcx.is_foreign_item(did) => {
-                match tcx.const_eval_poly(did) {
-                    Ok(ConstValue::ByRef { alloc, .. }) => {
                 match tcx.eval_static_initializer(did) {
                     Ok(alloc) => {
                         write!(w, " (static: {}, ", tcx.def_path_str(did))?;
                         write_allocation_track_relocs(w, alloc)?;
                     }
-                    Ok(_) => {
-                        span_bug!(tcx.def_span(did), " static item without `ByRef` initializer")
-                    }
                     Err(_) => write!(
                         w,
                         " (static: {}, error during initializer evaluation)",

diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs
@@ -111,7  111,6 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi
 use rustc_infer::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind};
 use rustc_infer::infer::{InferCtxt, InferOk, InferResult, RegionVariableOrigin, TyCtxtInferExt};
 use rustc_middle::hir::map::blocks::FnLikeNode;
-use rustc_middle::mir::interpret::ConstValue;
 use rustc_middle::ty::adjustment::{
     Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability,
 };
@@ -2070,16  2069,15 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt<'_>, id: LocalDefId, span: S
     // `#[link_section]` may contain arbitrary, or even undefined bytes, but it is
     // the consumer's responsibility to ensure all bytes that have been read
     // have defined values.
-    match tcx.const_eval_poly(id.to_def_id()) {
-        Ok(ConstValue::ByRef { alloc, .. }) => {
     match tcx.eval_static_initializer(id.to_def_id()) {
         Ok(alloc) => {
             if alloc.relocations().len() != 0 {
                 let msg = "statics with a custom `#[link_section]` must be a \
                            simple list of bytes on the wasm target with no \
                            extra levels of indirection such as references";
                 tcx.sess.span_err(span, msg);
             }
         }
-        Ok(_) => bug!("Matching on non-ByRef static"),
         Err(_) => {}
     }
 }

diff --git a/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr b/src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr
@@ -36,7  36,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:37:5
    |
 LL |     const I32_REF_U64_UNION: u64 = unsafe { Nonsense { int_32_ref: &3 }.uint_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc22, but expected initialized plain (non-pointer) bytes
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc18, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -76,7  76,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:52:5
    |
 LL |     const I32_REF_I64_UNION: i64 = unsafe { Nonsense { int_32_ref: &3 }.int_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc47, but expected initialized plain (non-pointer) bytes
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc38, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -100,7  100,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:61:5
    |
 LL |     const I32_REF_F64_UNION: f64 = unsafe { Nonsense { int_32_ref: &3 }.float_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc62, but expected initialized plain (non-pointer) bytes
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc50, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -148,7  148,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:79:5
    |
 LL |     const STR_U64_UNION: u64 = unsafe { Nonsense { stringy: "3" }.uint_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc86, but expected initialized plain (non-pointer) bytes
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc71, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -188,7  188,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:94:5
    |
 LL |     const STR_I64_UNION: i64 = unsafe { Nonsense { stringy: "3" }.int_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc101, but expected initialized plain (non-pointer) bytes
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc86, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -212,7  212,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:103:5
    |
 LL |     const STR_F64_UNION: f64 = unsafe { Nonsense { stringy: "3" }.float_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc110, but expected initialized plain (non-pointer) bytes
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc95, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 

diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr
@@ -18,7  18,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/ub-enum.rs:30:1
    |
 LL | const BAD_ENUM_WRAPPED: Wrap<Enum> = unsafe { mem::transmute(&1) };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc13 at .0.<enum-tag>, but expected initialized plain (non-pointer) bytes
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc12 at .0.<enum-tag>, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -34,15  34,15 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/ub-enum.rs:44:1
    |
 LL | const BAD_ENUM2_PTR: Enum2 = unsafe { mem::transmute(&0) };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc20 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc18 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
 error[E0080]: it is undefined behavior to use this value
   --> $DIR/ub-enum.rs:47:1
    |
 LL | const BAD_ENUM2_WRAPPED: Wrap<Enum2> = unsafe { mem::transmute(&0) };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc25 at .0.<enum-tag>, but expected initialized plain (non-pointer) bytes
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc22 at .0.<enum-tag>, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
@@ -58,7  58,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/ub-enum.rs:60:1
    |
 LL | const BAD_ENUM2_OPTION_PTR: Option<Enum2> = unsafe { mem::transmute(&0) };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc32 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to alloc28 at .<enum-tag>, but expected initialized plain (non-pointer) bytes
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 

diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr
@@ -13,7  13,7 @@ LL | / const OUT_OF_BOUNDS_PTR: NonNull<u8> = { unsafe {
 LL | |     let ptr: &[u8; 256] = mem::transmute(&0u8); // &0 gets promoted so it does not dangle
 LL | |     // Use address-of-element for pointer arithmetic. This could wrap around to NULL!
 LL | |     let out_of_bounds_ptr = &ptr[255];
-   | |                             ^^^^^^^^^ memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of alloc11 which has size 1
    | |                             ^^^^^^^^^ memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of alloc10 which has size 1
 LL | |     mem::transmute(out_of_bounds_ptr)
 LL | | } };
    | |____-