don't force all slice-typed ConstValue to be ConstValue::Slice

1 files changed, 29 insertions, 36 deletions

diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs
index d4b7d6f4e29..454baf2a745 100644
--- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs
+++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs
@@ -18,9 +18,9 @@ use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter};
 use crate::errors;
 use crate::interpret::eval_nullary_intrinsic;
 use crate::interpret::{
-    intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId,
-    Immediate, InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy,
-    RefTracking, StackPopCleanup,
+    intern_const_alloc_recursive, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId, Immediate,
+    InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking,
+    StackPopCleanup,
 };
 
 // Returns a pointer to where the result lives
@@ -105,8 +105,7 @@ pub(super) fn mk_eval_cx<'mir, 'tcx>(
     )
 }
 
-/// This function converts an interpreter value into a constant that is meant for use in the
-/// type system.
+/// This function converts an interpreter value into a MIR constant.
 #[instrument(skip(ecx), level = "debug")]
 pub(super) fn op_to_const<'tcx>(
     ecx: &CompileTimeEvalContext<'_, 'tcx>,
@@ -117,28 +116,25 @@ pub(super) fn op_to_const<'tcx>(
         return ConstValue::ZeroSized;
     }
 
-    // We do not have value optimizations for everything.
-    // Only scalars and slices, since they are very common.
-    let try_as_immediate = match op.layout.abi {
+    // All scalar types should be stored as `ConstValue::Scalar`. This is needed to make
+    // `ConstValue::try_to_scalar` efficient; we want that to work for *all* constants of scalar
+    // type (it's used throughout the compiler and having it work just on literals is not enough)
+    // and we want it to be fast (i.e., don't go to an `Allocation` and reconstruct the `Scalar`
+    // from its byte-serialized form).
+    let force_as_immediate = match op.layout.abi {
         Abi::Scalar(abi::Scalar::Initialized { .. }) => true,
-        Abi::ScalarPair(..) => match op.layout.ty.kind() {
-            ty::Ref(_, inner, _) => match *inner.kind() {
-                ty::Slice(elem) => elem == ecx.tcx.types.u8,
-                ty::Str => true,
-                _ => false,
-            },
-            _ => false,
-        },
+        // We don't *force* `ConstValue::Slice` for `ScalarPair`. This has the advantage that if the
+        // input `op` is a place, then turning it into a `ConstValue` and back into a `OpTy` will
+        // not have to generate any duplicate allocations (we preserve the original `AllocId` in
+        // `ConstValue::Indirect`). It means accessing the contents of a slice can be slow (since
+        // they can be stored as `ConstValue::Indirect`), but that's not relevant since we barely
+        // ever have to do this. (`try_get_slice_bytes_for_diagnostics` exists to provide this
+        // functionality.)
         _ => false,
     };
-    let immediate = if try_as_immediate {
+    let immediate = if force_as_immediate {
         Right(ecx.read_immediate(op).expect("normalization works on validated constants"))
     } else {
-        // It is guaranteed that any non-slice scalar pair is actually `Indirect` here.
-        // When we come back from raw const eval, we are always by-ref. The only way our op here is
-        // by-val is if we are in destructure_mir_constant, i.e., if this is (a field of) something that we
-        // "tried to make immediate" before. We wouldn't do that for non-slice scalar pairs or
-        // structs containing such.
         op.as_mplace_or_imm()
     };
 
@@ -151,25 +147,22 @@ pub(super) fn op_to_const<'tcx>(
             let alloc_id = alloc_id.expect("cannot have `fake` place fot non-ZST type");
             ConstValue::Indirect { alloc_id, offset }
         }
-        // see comment on `let try_as_immediate` above
+        // see comment on `let force_as_immediate` above
         Right(imm) => match *imm {
             Immediate::Scalar(x) => ConstValue::Scalar(x),
             Immediate::ScalarPair(a, b) => {
                 debug!("ScalarPair(a: {:?}, b: {:?})", a, b);
+                // FIXME: assert that this has an appropriate type.
+                // Currently we actually get here for non-[u8] slices during valtree construction!
+                let msg = "`op_to_const` on an immediate scalar pair must only be used on slice references to actually allocated memory";
                 // We know `offset` is relative to the allocation, so we can use `into_parts`.
-                let (data, start) = match a.to_pointer(ecx).unwrap().into_parts() {
-                    (Some(alloc_id), offset) => {
-                        (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
-                    }
-                    (None, _offset) => (
-                        ecx.tcx.mk_const_alloc(Allocation::from_bytes_byte_aligned_immutable(
-                            b"" as &[u8],
-                        )),
-                        0,
-                    ),
-                };
-                let len = b.to_target_usize(ecx).unwrap();
-                let start = start.try_into().unwrap();
+                // We use `ConstValue::Slice` so that we don't have to generate an allocation for
+                // `ConstValue::Indirect` here.
+                let (alloc_id, offset) = a.to_pointer(ecx).expect(msg).into_parts();
+                let alloc_id = alloc_id.expect(msg);
+                let data = ecx.tcx.global_alloc(alloc_id).unwrap_memory();
+                let start = offset.bytes_usize();
+                let len = b.to_target_usize(ecx).expect(msg);
                 let len: usize = len.try_into().unwrap();
                 ConstValue::Slice { data, start, end: start + len }
             }