consistently treat None-tagged pointers as ints; get rid of some deprecated Scalar methods

Author: RalfJung

compiler/rustc_codegen_llvm/src/common.rs

@@ -243,7 +243,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                     self.const_bitcast(llval, llty)
                 }
             }
-            Scalar::Ptr(ptr) => {
+            Scalar::Ptr(ptr, _size) => {
                 let (alloc_id, offset) = ptr.into_parts();
                 let (base_addr, base_addr_space) = match self.tcx.global_alloc(alloc_id) {
                     GlobalAlloc::Memory(alloc) => {

compiler/rustc_codegen_llvm/src/consts.rs

@@ -11,7 +11,7 @@ use rustc_codegen_ssa::traits::*;
 use rustc_hir::def_id::DefId;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
 use rustc_middle::mir::interpret::{
-    read_target_uint, Allocation, ErrorHandled, GlobalAlloc, Pointer,
+    read_target_uint, Allocation, ErrorHandled, GlobalAlloc, Pointer, Scalar as InterpScalar,
 };
 use rustc_middle::mir::mono::MonoItem;
 use rustc_middle::ty::{self, Instance, Ty};
@@ -55,7 +55,10 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll
         };
 
         llvals.push(cx.scalar_to_backend(
-            Pointer::new(alloc_id, Size::from_bytes(ptr_offset)).into(),
+            InterpScalar::from_pointer(
+                Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
+                &cx.tcx,
+            ),
             &Scalar { value: Primitive::Pointer, valid_range: 0..=!0 },
             cx.type_i8p_ext(address_space),
         ));

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -90,10 +90,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
                     Abi::ScalarPair(ref a, _) => a,
                     _ => bug!("from_const: invalid ScalarPair layout: {:#?}", layout),
                 };
-                let a = Scalar::from(Pointer::new(
-                    bx.tcx().create_memory_alloc(data),
-                    Size::from_bytes(start),
-                ));
+                let a = Scalar::from_pointer(
+                    Pointer::new(bx.tcx().create_memory_alloc(data), Size::from_bytes(start)),
+                    &bx.tcx(),
+                );
                 let a_llval = bx.scalar_to_backend(
                     a,
                     a_scalar,

compiler/rustc_middle/src/mir/interpret/allocation.rs

@@ -333,7 +333,7 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
             // Maybe a pointer.
             if let Some(&prov) = self.relocations.get(&range.start) {
                 let ptr = Pointer::new(prov, Size::from_bytes(bits));
-                return Ok(ScalarMaybeUninit::Scalar(ptr.into()));
+                return Ok(ScalarMaybeUninit::from_pointer(ptr, cx));
             }
         }
         // We don't. Just return the bits.
@@ -363,7 +363,7 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
             }
         };
 
-        let (bytes, provenance) = match val.to_bits_or_ptr(range.size, cx) {
+        let (bytes, provenance) = match val.to_bits_or_ptr(range.size) {
             Err(val) => {
                 let (provenance, offset) = val.into_parts();
                 (u128::from(offset.bytes()), Some(provenance))

compiler/rustc_middle/src/mir/interpret/pointer.rs

@@ -87,7 +87,7 @@ impl<T: HasDataLayout> PointerArithmetic for T {}
 /// mostly opaque; the `Machine` trait extends it with some more operations that also have access to
 /// some global state.
 pub trait Provenance: Copy {
-    /// Says whether the `offset` field of `Pointer` is the actual physical address.
+    /// Says whether the `offset` field of `Pointer`s with this provenance is the actual physical address.
     /// If `true, ptr-to-int casts work by simply discarding the provenance.
     /// If `false`, ptr-to-int casts are not supported.
     const OFFSET_IS_ADDR: bool;

compiler/rustc_middle/src/mir/interpret/value.rs

@@ -128,7 +128,11 @@ pub enum Scalar<Tag = AllocId> {
     /// A pointer into an `Allocation`. An `Allocation` in the `memory` module has a list of
     /// relocations, but a `Scalar` is only large enough to contain one, so we just represent the
     /// relocation and its associated offset together as a `Pointer` here.
-    Ptr(Pointer<Tag>),
+    ///
+    /// We also store the size of the pointer, such that a `Scalar` always knows how big it is.
+    /// The size is always the pointer size of the current target, but this is not information
+    /// that we always have readily available.
+    Ptr(Pointer<Tag>, u8),
 }
 
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
@@ -139,7 +143,7 @@ pub enum Scalar<Tag = AllocId> {
 impl<Tag: Provenance> fmt::Debug for Scalar<Tag> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Scalar::Ptr(ptr) => write!(f, "{:?}", ptr),
+            Scalar::Ptr(ptr, _size) => write!(f, "{:?}", ptr),
             Scalar::Int(int) => write!(f, "{:?}", int),
         }
     }
@@ -148,7 +152,7 @@ impl<Tag: Provenance> fmt::Debug for Scalar<Tag> {
 impl<Tag: Provenance> fmt::Display for Scalar<Tag> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Scalar::Ptr(ptr) => write!(f, "pointer to {:?}", ptr),
+            Scalar::Ptr(ptr, _size) => write!(f, "pointer to {:?}", ptr),
             Scalar::Int(int) => write!(f, "{:?}", int),
         }
     }
@@ -168,13 +172,6 @@ impl<Tag> From<Double> for Scalar<Tag> {
     }
 }
 
-impl<Tag> From<Pointer<Tag>> for Scalar<Tag> {
-    #[inline(always)]
-    fn from(ptr: Pointer<Tag>) -> Self {
-        Scalar::Ptr(ptr)
-    }
-}
-
 impl<Tag> From<ScalarInt> for Scalar<Tag> {
     #[inline(always)]
     fn from(ptr: ScalarInt) -> Self {
@@ -185,21 +182,26 @@ impl<Tag> From<ScalarInt> for Scalar<Tag> {
 impl<'tcx, Tag> Scalar<Tag> {
     pub const ZST: Self = Scalar::Int(ScalarInt::ZST);
 
-    #[inline]
-    pub fn null_ptr(cx: &impl HasDataLayout) -> Self {
-        Scalar::Int(ScalarInt::null(cx.data_layout().pointer_size))
+    #[inline(always)]
+    pub fn from_pointer(ptr: Pointer<Tag>, cx: &impl HasDataLayout) -> Self {
+        Scalar::Ptr(ptr, u8::try_from(cx.pointer_size().bytes()).unwrap())
     }
 
     /// Create a Scalar from a pointer with an `Option<_>` tag (where `None` represents a plain integer).
     pub fn from_maybe_pointer(ptr: Pointer<Option<Tag>>, cx: &impl HasDataLayout) -> Self {
         match ptr.into_parts() {
-            (Some(tag), offset) => Scalar::Ptr(Pointer::new(tag, offset)),
+            (Some(tag), offset) => Scalar::from_pointer(Pointer::new(tag, offset), cx),
             (None, offset) => {
                 Scalar::Int(ScalarInt::try_from_uint(offset.bytes(), cx.pointer_size()).unwrap())
             }
         }
     }
 
+    #[inline]
+    pub fn null_ptr(cx: &impl HasDataLayout) -> Self {
+        Scalar::Int(ScalarInt::null(cx.pointer_size()))
+    }
+
     #[inline(always)]
     fn ptr_op(
         self,
@@ -209,7 +211,10 @@ impl<'tcx, Tag> Scalar<Tag> {
     ) -> InterpResult<'tcx, Self> {
         match self {
             Scalar::Int(int) => Ok(Scalar::Int(int.ptr_sized_op(dl, f_int)?)),
-            Scalar::Ptr(ptr) => Ok(Scalar::Ptr(f_ptr(ptr)?)),
+            Scalar::Ptr(ptr, sz) => {
+                debug_assert_eq!(u64::from(sz), dl.pointer_size().bytes());
+                Ok(Scalar::Ptr(f_ptr(ptr)?, sz))
+            }
         }
     }
 
@@ -334,59 +339,18 @@ impl<'tcx, Tag> Scalar<Tag> {
     /// This is almost certainly not the method you want!  You should dispatch on the type
     /// and use `to_{u8,u16,...}`/`scalar_to_ptr` to perform ptr-to-int / int-to-ptr casts as needed.
     ///
-    /// This method only exists for the benefit of low-level memory operations
-    /// as well as the implementation of the above methods.
+    /// This method only exists for the benefit of low-level memory operations.
     #[inline]
-    pub fn to_bits_or_ptr(
-        self,
-        target_size: Size,
-        cx: &impl HasDataLayout,
-    ) -> Result<u128, Pointer<Tag>> {
+    pub fn to_bits_or_ptr(self, target_size: Size) -> Result<u128, Pointer<Tag>> {
         assert_ne!(target_size.bytes(), 0, "you should never look at the bits of a ZST");
         match self {
             Scalar::Int(int) => Ok(int.assert_bits(target_size)),
-            Scalar::Ptr(ptr) => {
-                assert_eq!(target_size, cx.data_layout().pointer_size);
+            Scalar::Ptr(ptr, sz) => {
+                assert_eq!(target_size.bytes(), u64::from(sz));
                 Err(ptr)
             }
         }
     }
-
-    /// Do not call this method! It does not do ptr-to-int casts when needed.
-    #[inline(always)]
-    pub fn assert_bits(self, target_size: Size) -> u128 {
-        self.assert_int().assert_bits(target_size)
-    }
-
-    /// Do not call this method! It does not do ptr-to-int casts when needed.
-    #[inline]
-    pub fn assert_int(self) -> ScalarInt {
-        match self {
-            Scalar::Ptr(_) => bug!("expected an int but got an abstract pointer"),
-            Scalar::Int(int) => int,
-        }
-    }
-
-    /// Do not call this method! It does not do int-to-ptr casts when needed.
-    #[inline]
-    pub fn assert_ptr(self) -> Pointer<Tag> {
-        match self {
-            Scalar::Ptr(p) => p,
-            Scalar::Int { .. } => bug!("expected a Pointer but got Raw bits"),
-        }
-    }
-
-    /// Do not call this method!  Dispatch based on the type instead.
-    #[inline]
-    pub fn is_bits(self) -> bool {
-        matches!(self, Scalar::Int { .. })
-    }
-
-    /// Do not call this method!  Dispatch based on the type instead.
-    #[inline]
-    pub fn is_ptr(self) -> bool {
-        matches!(self, Scalar::Ptr(_))
-    }
 }
 
 impl<'tcx, Tag: Provenance> Scalar<Tag> {
@@ -396,7 +360,7 @@ impl<'tcx, Tag: Provenance> Scalar<Tag> {
     #[inline]
     pub fn erase_for_fmt(self) -> Scalar {
         match self {
-            Scalar::Ptr(ptr) => Scalar::Ptr(ptr.erase_for_fmt()),
+            Scalar::Ptr(ptr, sz) => Scalar::Ptr(ptr.erase_for_fmt(), sz),
             Scalar::Int(int) => Scalar::Int(int),
         }
     }
@@ -405,25 +369,45 @@ impl<'tcx, Tag: Provenance> Scalar<Tag> {
     /// likely want to use instead.
     ///
     /// Will perform ptr-to-int casts if needed and possible.
+    #[inline]
+    pub fn try_to_int(self) -> Option<ScalarInt> {
+        match self {
+            Scalar::Int(int) => Some(int),
+            Scalar::Ptr(ptr, sz) => {
+                if Tag::OFFSET_IS_ADDR {
+                    Some(
+                        ScalarInt::try_from_uint(ptr.offset.bytes(), Size::from_bytes(sz)).unwrap(),
+                    )
+                } else {
+                    None
+                }
+            }
+        }
+    }
+
+    #[inline(always)]
+    pub fn assert_int(self) -> ScalarInt {
+        self.try_to_int().unwrap()
+    }
+
     #[inline]
     pub fn to_bits(self, target_size: Size) -> InterpResult<'tcx, u128> {
         assert_ne!(target_size.bytes(), 0, "you should never look at the bits of a ZST");
-        match self {
-            Scalar::Int(int) => int.to_bits(target_size).map_err(|size| {
+        self.try_to_int()
+            .ok_or_else(|| err_unsup!(ReadPointerAsBytes))?
+            .to_bits(target_size)
+            .map_err(|size| {
                 err_ub!(ScalarSizeMismatch {
                     target_size: target_size.bytes(),
                     data_size: size.bytes(),
                 })
                 .into()
-            }),
-            Scalar::Ptr(ptr) => {
-                if Tag::OFFSET_IS_ADDR {
-                    Ok(ptr.offset.bytes().into())
-                } else {
-                    throw_unsup!(ReadPointerAsBytes)
-                }
-            }
-        }
+            })
+    }
+
+    #[inline(always)]
+    pub fn assert_bits(self, target_size: Size) -> u128 {
+        self.to_bits(target_size).unwrap()
     }
 
     pub fn to_bool(self) -> InterpResult<'tcx, bool> {
@@ -547,13 +531,6 @@ impl<Tag> From<Scalar<Tag>> for ScalarMaybeUninit<Tag> {
     }
 }
 
-impl<Tag> From<Pointer<Tag>> for ScalarMaybeUninit<Tag> {
-    #[inline(always)]
-    fn from(s: Pointer<Tag>) -> Self {
-        ScalarMaybeUninit::Scalar(s.into())
-    }
-}
-
 // We want the `Debug` output to be readable as it is used by `derive(Debug)` for
 // all the Miri types.
 impl<Tag: Provenance> fmt::Debug for ScalarMaybeUninit<Tag> {
@@ -575,6 +552,16 @@ impl<Tag: Provenance> fmt::Display for ScalarMaybeUninit<Tag> {
 }
 
 impl<Tag> ScalarMaybeUninit<Tag> {
+    #[inline]
+    pub fn from_pointer(ptr: Pointer<Tag>, cx: &impl HasDataLayout) -> Self {
+        ScalarMaybeUninit::Scalar(Scalar::from_pointer(ptr, cx))
+    }
+
+    #[inline]
+    pub fn from_maybe_pointer(ptr: Pointer<Option<Tag>>, cx: &impl HasDataLayout) -> Self {
+        ScalarMaybeUninit::Scalar(Scalar::from_maybe_pointer(ptr, cx))
+    }
+
     #[inline]
     pub fn check_init(self) -> InterpResult<'static, Scalar<Tag>> {
         match self {

compiler/rustc_middle/src/mir/mod.rs

@@ -2458,7 +2458,7 @@ pub enum ConstantKind<'tcx> {
 impl Constant<'tcx> {
     pub fn check_static_ptr(&self, tcx: TyCtxt<'_>) -> Option<DefId> {
         match self.literal.const_for_ty()?.val.try_to_scalar() {
-            Some(Scalar::Ptr(ptr)) => match tcx.global_alloc(ptr.provenance) {
+            Some(Scalar::Ptr(ptr, _size)) => match tcx.global_alloc(ptr.provenance) {
                 GlobalAlloc::Static(def_id) => {
                     assert!(!tcx.is_thread_local_static(def_id));
                     Some(def_id)

compiler/rustc_middle/src/ty/print/pretty.rs

@@ -974,7 +974,7 @@ pub trait PrettyPrinter<'tcx>:
         print_ty: bool,
     ) -> Result<Self::Const, Self::Error> {
         match scalar {
-            Scalar::Ptr(ptr) => self.pretty_print_const_scalar_ptr(ptr, ty, print_ty),
+            Scalar::Ptr(ptr, _size) => self.pretty_print_const_scalar_ptr(ptr, ty, print_ty),
             Scalar::Int(int) => self.pretty_print_const_scalar_int(int, ty, print_ty),
         }
     }

compiler/rustc_middle/src/ty/relate.rs

@@ -595,7 +595,10 @@ fn check_const_value_eq<R: TypeRelation<'tcx>>(
         (ConstValue::Scalar(Scalar::Int(a_val)), ConstValue::Scalar(Scalar::Int(b_val))) => {
             a_val == b_val
         }
-        (ConstValue::Scalar(Scalar::Ptr(a_val)), ConstValue::Scalar(Scalar::Ptr(b_val))) => {
+        (
+            ConstValue::Scalar(Scalar::Ptr(a_val, _a_size)),
+            ConstValue::Scalar(Scalar::Ptr(b_val, _b_size)),
+        ) => {
             a_val == b_val
                 || match (tcx.global_alloc(a_val.provenance), tcx.global_alloc(b_val.provenance)) {
                     (GlobalAlloc::Function(a_instance), GlobalAlloc::Function(b_instance)) => {

compiler/rustc_middle/src/ty/vtable.rs

@@ -1,6 +1,6 @@
 use std::convert::TryFrom;
 
-use crate::mir::interpret::{alloc_range, AllocId, Allocation, Pointer, Scalar};
+use crate::mir::interpret::{alloc_range, AllocId, Allocation, Pointer, Scalar, ScalarMaybeUninit};
 use crate::ty::fold::TypeFoldable;
 use crate::ty::{self, DefId, SubstsRef, Ty, TyCtxt};
 use rustc_ast::Mutability;
@@ -74,7 +74,7 @@ impl<'tcx> TyCtxt<'tcx> {
                     let instance = ty::Instance::resolve_drop_in_place(tcx, ty);
                     let fn_alloc_id = tcx.create_fn_alloc(instance);
                     let fn_ptr = Pointer::from(fn_alloc_id);
-                    fn_ptr.into()
+                    ScalarMaybeUninit::from_pointer(fn_ptr, &tcx)
                 }
                 VtblEntry::MetadataSize => Scalar::from_uint(size, ptr_size).into(),
                 VtblEntry::MetadataAlign => Scalar::from_uint(align, ptr_size).into(),
@@ -90,7 +90,7 @@ impl<'tcx> TyCtxt<'tcx> {
                             .polymorphize(tcx);
                     let fn_alloc_id = tcx.create_fn_alloc(instance);
                     let fn_ptr = Pointer::from(fn_alloc_id);
-                    fn_ptr.into()
+                    ScalarMaybeUninit::from_pointer(fn_ptr, &tcx)
                 }
             };
             vtable