StringValue type

Summary:
`StringValue` is a wrapper to `Value` which asserts that contained value is `StarlarkStr`.

(Similar to `FrozenStringValue` which is logically a wrapper for `FrozenValue` which asserts that contained value is `StarlarkStr`).

Used in the following diff D30884359.

In theory `StringValue` can be used for faster operations on strings (e. g. obtaining a string from `StringValue` is cheaper than obtaining a string from `Value`), but there's no obvious applications for that at the moment.

Reviewed By: ndmitchell

Differential Revision: D30884361

fbshipit-source-id: 21b632f2a6a4362eb05dda35b0e2a12162397faf

Author: stepancheg

starlark/src/values/alloc_value.rs

@@ -17,7 +17,7 @@
 
 //! This mod defines utilities to easily create Rust values as Starlark values.
 
-use crate::values::{layout::Value, FrozenHeap, FrozenValue, Heap};
+use crate::values::{layout::Value, FrozenHeap, FrozenStringValue, FrozenValue, Heap, StringValue};
 
 /// Trait for things that can be created on a [`Heap`] producing a [`Value`].
 ///
@@ -69,11 +69,21 @@ impl FrozenHeap {
     pub fn alloc<T: AllocFrozenValue>(&self, val: T) -> FrozenValue {
         val.alloc_frozen_value(self)
     }
+
+    #[allow(dead_code)] // TODO: remove in the following diff
+    pub(crate) fn alloc_string_value(&self, s: &str) -> FrozenStringValue {
+        unsafe { FrozenStringValue::new_unchecked(self.alloc_str(s)) }
+    }
 }
 
 impl Heap {
     /// Allocate a new value on a [`Heap`].
     pub fn alloc<'v, T: AllocValue<'v>>(&'v self, x: T) -> Value<'v> {
         x.alloc_value(self)
     }
+
+    #[allow(dead_code)] // TODO: remove in the following diff
+    pub(crate) fn alloc_string_value<'v>(&'v self, s: &str) -> StringValue<'v> {
+        unsafe { StringValue::new_unchecked(self.alloc_str(s)) }
+    }
 }

starlark/src/values/layout/arena.rs

@@ -319,8 +319,8 @@ impl AValueHeader {
 
     pub(crate) fn unpack<'v>(&'v self) -> &'v dyn AValue<'v> {
         unsafe {
-            let self_repr = self as *const AValueHeader as *const AValueRepr<()>;
-            let res = &*(from_raw_parts(&(*self_repr).payload, self.0));
+            let self_repr = self.as_repr::<()>();
+            let res = &*(from_raw_parts(&self_repr.payload, self.0));
             mem::transmute::<&'v dyn AValue<'static>, &'v dyn AValue<'v>>(res)
         }
     }
@@ -362,6 +362,11 @@ impl AValueHeader {
         let p = self as *const AValueHeader as *mut u8;
         p.add(mem::size_of::<AValueHeader>() + n)
     }
+
+    /// Cast header pointer to repr pointer.
+    pub(crate) unsafe fn as_repr<T>(&self) -> &AValueRepr<T> {
+        &*(self as *const AValueHeader as *const AValueRepr<T>)
+    }
 }
 
 impl<T> AValueRepr<T> {

starlark/src/values/layout/constant.rs

@@ -18,9 +18,17 @@
 use crate::values::{
     layout::{arena::AValueRepr, avalue::VALUE_STR_A_VALUE_PTR, value::FrozenValue},
     string::StarlarkStr,
+    Freezer, Trace, Tracer, Value,
+};
+use gazebo::{
+    coerce::{Coerce, CoerceKey},
+    prelude::*,
+};
+use std::{
+    fmt,
+    fmt::{Debug, Formatter},
+    intrinsics::copy_nonoverlapping,
 };
-use gazebo::prelude::*;
-use std::intrinsics::copy_nonoverlapping;
 
 /// A constant string that can be converted to a [`FrozenValue`].
 #[repr(C)] // Must match this layout on the heap
@@ -71,13 +79,88 @@ impl<const N: usize> ConstFrozenStringN<N> {
 /// assert_eq!(Some("magic"), fv.to_value().unpack_str());
 /// ```
 #[derive(Copy, Clone, Dupe)]
+#[repr(C)]
 pub struct FrozenStringValue(&'static AValueRepr<StarlarkStr>);
 
+impl Debug for FrozenStringValue {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("FrozenStringValue")
+            .field(&self.unpack())
+            .finish()
+    }
+}
+
+/// Wrapper for a [`Value`] which can only contain a [`StarlarkStr`].
+#[derive(Copy, Clone, Dupe, Debug)]
+#[repr(C)]
+pub struct StringValue<'v>(Value<'v>);
+
+unsafe impl<'v> Coerce<StringValue<'v>> for FrozenStringValue {}
+unsafe impl<'v> CoerceKey<StringValue<'v>> for FrozenStringValue {}
+unsafe impl<'v> Coerce<StringValue<'v>> for StringValue<'v> {}
+unsafe impl<'v> CoerceKey<StringValue<'v>> for StringValue<'v> {}
+
 impl FrozenStringValue {
     /// Obtain the [`FrozenValue`] for a [`FrozenStringValue`].
     pub fn unpack(self) -> FrozenValue {
         FrozenValue::new_repr(self.0)
     }
+
+    /// Construct without a check that the value contains a string.
+    ///
+    /// If passed value does not contain a string, it may lead to memory corruption.
+    pub(crate) unsafe fn new_unchecked(value: FrozenValue) -> FrozenStringValue {
+        debug_assert!(value.unpack_str().is_some());
+        FrozenStringValue(&*(value.0.ptr_value() as *const AValueRepr<StarlarkStr>))
+    }
+}
+
+#[allow(dead_code)] // TODO: remove in the following diff
+impl<'v> StringValue<'v> {
+    /// Construct without a check that the value contains a string.
+    ///
+    /// If passed value does not contain a string, it may lead to memory corruption.
+    pub(crate) unsafe fn new_unchecked(value: Value<'v>) -> StringValue<'v> {
+        debug_assert!(value.unpack_str().is_some());
+        StringValue(value)
+    }
+
+    /// Construct from a value. Returns [`None`] if a value does not contain a string.
+    pub(crate) fn new(value: Value<'v>) -> Option<StringValue<'v>> {
+        if value.unpack_str().is_some() {
+            Some(StringValue(value))
+        } else {
+            None
+        }
+    }
+
+    pub(crate) fn unpack_starlark_str(self) -> &'v StarlarkStr {
+        debug_assert!(self.0.unpack_str().is_some());
+        unsafe { &self.0.0.unpack_ptr_no_int_unchecked().as_repr().payload }
+    }
+
+    pub(crate) fn to_value(self) -> Value<'v> {
+        self.0
+    }
+
+    /// Convert a value to a [`FrozenValue`] using a supplied [`Freezer`].
+    pub(crate) fn freeze(self, freezer: &Freezer) -> anyhow::Result<FrozenStringValue> {
+        Ok(unsafe { FrozenStringValue::new_unchecked(freezer.freeze(self.0)?) })
+    }
+}
+
+/// Common type for [`StringValue`] and [`FrozenStringValue`].
+pub(crate) trait StringValueLike<'v>: Debug + Coerce<StringValue<'v>> {}
+
+impl<'v> StringValueLike<'v> for StringValue<'v> {}
+
+impl<'v> StringValueLike<'v> for FrozenStringValue {}
+
+unsafe impl<'v> Trace<'v> for StringValue<'v> {
+    fn trace(&mut self, tracer: &Tracer<'v>) {
+        self.0.trace(tracer);
+        debug_assert!(self.0.unpack_str().is_some());
+    }
 }
 
 /// Create a [`FrozenStringValue`].

starlark/src/values/layout/mod.rs

@@ -27,7 +27,9 @@ mod pointer_i32;
 mod value;
 mod value_captured;
 
-pub use constant::{ConstFrozenStringN, FrozenStringValue};
+#[allow(unused_imports)] // TODO: remove in the following diff
+pub(crate) use constant::StringValueLike;
+pub use constant::{ConstFrozenStringN, FrozenStringValue, StringValue};
 pub use heap::{Freezer, FrozenHeap, FrozenHeapRef, Heap, Tracer};
 pub(crate) use pointer_i32::PointerI32;
 pub use value::{FrozenValue, Value};

starlark/src/values/layout/pointer.rs

@@ -133,6 +133,14 @@ impl<'p, P> Pointer<'p, P> {
         }
     }
 
+    /// Unpack pointer when it is known to be not an integer.
+    #[allow(dead_code)] // TODO: remove in the following diff
+    pub(crate) unsafe fn unpack_ptr_no_int_unchecked(self) -> &'p P {
+        let p = self.pointer.get();
+        debug_assert!(p & TAG_INT == 0);
+        untag_pointer(p)
+    }
+
     pub fn ptr_eq(self, other: Pointer<'_, P>) -> bool {
         self.pointer == other.pointer
     }