Low-dependency ref-cast

[sffc]

Previous attempt: #5101
Also see: #6915

We have a decent amount of unsafe code involving ref-cast, which, if eliminated, would help make some crates be fully safe.

We could use the ref-cast crate, but it seems like a heavy dependency (involving syn) to replace just one transmute call.

#5101 was my first attempt at making macro_rules for this. My recollection is that one of the main issues with that approach was that it didn't eliminate the unsafe, it just shifted it around. Would moving that code to a separate crate be a viable option?

Another approach I was thinking was to check the fields by their type, as shown here.

What do we think about what to do here?

1. Keep using transmute and allow such call sites as in Advertise that zerotrie uses safe Rust #6915
2. Use ref-cast and accept the extra cost
3. Use ref-cast when the feature is enabled and transmute when the feature is not enabled
4. Add our own crate doing something like Add icu4x_shared with some helpers #5101 or the above playground
5. Something else?

@Manishearth

[sffc]

Labeling as discuss-priority in order to get a high-level signal for the next steps here.

[Manishearth]

Can you list the current usages, ideally categorized by type of operation? Given that we don't actually use the ref cast crate, grepping for one type of transmute seems tricky, and since you've been looking at this you probably have a better idea as to what we try to do with these.

I know for the ULE cases we had hopes for adding macros.

Keep using transmute and allow such call sites as in #6915

I don't understand what you mean by "allow such call sites as in #6915". We already allow them? My comment in that thread is not pushing back against the unsafe, I'm just asking for more precise documentation.

My general preference, without having seen the call sites in a while, is to keep using transmute. Transparent conversions tend to be straightforward unsafe usages, easy to review. On the other hand, custom unsafe abstractions that make transparent conversions "safe" are complex and much harder to review. In this case, the simple but more profligate code is my preference, and we can wait for Project Safe Transmute.

I'd be okay with depending on ref-cast, but this is dispreferred.

[Manishearth]

Another approach I was thinking was to check the fields by their type, as shown here.

I don't quite understand what this is doing

[sffc]

Another approach I was thinking was to check the fields by their type, as shown here.

I don't quite understand what this is doing

A macro can generate a function that checks that there is only one non-phantom field.

[Manishearth]

I still don't understand, that part is already checked by repr(transparent)? How would it enforce the layout requirement?

[Manishearth]

I can imagine some ways to static assert the layout requirement but AIUI it's UB to do this without (a) the type being a POD type or (b) an actual repr(transparent) annotation.

[sffc]

Can you list the current usages, ideally categorized by type of operation?

This is one of that handful of things I consider self-evident, but ok, here are some examples

MultiFieldsULE

#[repr(transparent)]
pub struct MultiFieldsULE<const LEN: usize, Format: VarZeroVecFormat>(
    VarZeroLengthlessSlice<[u8], Format>,
);
// ...
unsafe {
    // Safe since write_serializable_bytes produces a valid VarZeroLengthlessSlice buffer with the right format
    let slice = <VarZeroLengthlessSlice<[u8], Format>>::from_bytes_unchecked_mut(output);
    // safe since `Self` is transparent over VarZeroLengthlessSlice<[u8], Format>
    mem::transmute::<&mut VarZeroLengthlessSlice<[u8], Format>, &mut Self>(slice)
}

and a bunch more call sites in the zerovec crate which I don't feel a need to include here. Lots of casting between different VarULE types and subtypes. If adding the ~dozen additional zerovec call sites to this post would be important for your analysis, I will edit this post and add them.

ByteStr

#[repr(transparent)]
#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub(crate) struct ByteStr([u8]);
// ...
pub const fn from_byte_slice_with_value<'a, 'l>(
    input: &'l [(&'a [u8], usize)],
) -> &'l [(&'a ByteStr, usize)] {
    // Safety: [u8] and ByteStr have the same layout and invariants
    unsafe { core::mem::transmute(input) }
}

AsciiByte: however, ref-cast doesn't support slices. 🤯 🤷‍♂️ dtolnay/ref-cast#13

#[inline]
pub const fn from_ascii_bytes<const N: usize>(src: &[AsciiByte; N]) -> Self {
    Self::from_utf8::<N>(unsafe { core::mem::transmute::<&[AsciiByte; N], &[u8; N]>(src) })
}

PotentialUtf8

#[repr(transparent)]
#[derive(PartialEq, Eq, PartialOrd, Ord)]
#[allow(clippy::exhaustive_structs)] // transparent newtype
pub struct PotentialUtf8(pub [u8]);
// ...
#[inline]
pub const fn from_bytes(other: &[u8]) -> &Self {
    // Safety: PotentialUtf8 is transparent over [u8]
    unsafe { core::mem::transmute(other) }
}

DataMarkerAttributes

#[repr(transparent)]
pub struct DataMarkerAttributes {
    // Validated to be non-empty ASCII alphanumeric + hyphen + underscore
    value: str,
}
// ...
pub const fn try_from_utf8(code_units: &[u8]) -> Result<&Self, AttributeParseError> {
    let Ok(()) = Self::validate(code_units) else {
        return Err(AttributeParseError);
    };

    // SAFETY: `validate` requires a UTF-8 subset
    let s = unsafe { core::str::from_utf8_unchecked(code_units) };

    // SAFETY: `Self` has the same layout as `str`
    Ok(unsafe { &*(s as *const str as *const Self) })
}

Many of these call sites are supplemented with another one that deals with boxes, but unfortunately ref-cast decided that boxes are out of scope... dtolnay/ref-cast#44

ZeroTrie

#[repr(transparent)]
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "databake", derive(databake::Bake))]
#[cfg_attr(feature = "databake", databake(path = zerotrie))]
#[allow(clippy::exhaustive_structs)] // databake hidden fields
pub struct ZeroTrieSimpleAscii<Store: ?Sized> {
    #[doc(hidden)] // for databake, but there are no invariants
    pub store: Store,
}

impl<Store: ?Sized> ZeroTrieSimpleAscii<Store> {
    fn transparent_ref_from_store(s: &Store) -> &Self {
        unsafe {
            // Safety: Self is transparent over Store
            core::mem::transmute(s)
        }
    }
}

PluralElements

unsafe impl<V> VarULE for PluralElementsPackedULE<V>
where
    V: VarULE + ?Sized,
{
    unsafe fn from_bytes_unchecked(bytes: &[u8]) -> &Self {
        // Safety: the bytes are valid by trait invariant, and we are transparent over bytes
        core::mem::transmute(bytes)
    }
}

PatternItem

#[repr(transparent)]
pub struct GenericPatternItemULE([u8; 3]);
// ...
/// Converts this [`GenericPatternItemULE`] to a [`PatternItemULE`]
/// (if a Literal) or returns the placeholder value.
#[inline]
pub(crate) fn as_pattern_item_ule(&self) -> Result<&PatternItemULE, u8> {
    if Self::determine_field_from_u8(self.0[0]) {
        Err(self.0[2])
    } else {
        if cfg!(debug_assertions) {
            let GenericPatternItem::Literal(c) = GenericPatternItem::from_unaligned(*self)
            else {
                unreachable!("expected a literal!")
            };
            let pattern_item_ule = PatternItem::Literal(c).to_unaligned();
            debug_assert_eq!(self.0, pattern_item_ule.0);
        }
        // Safety: The two types are repr(transparent) over [u8; 3].
        // When a Literal, the two ULEs have the same repr,
        // as shown in the above assertion (and the class docs).
        Ok(unsafe { core::mem::transmute::<&GenericPatternItemULE, &PatternItemULE>(self) })
    }
}

CodePointTrie

#[repr(transparent)]
pub struct FastCodePointTrie<'trie, T: TrieValue> {
    inner: CodePointTrie<'trie, T>,
}
// On CodePointTrie:
/// Obtains a reference to this trie as a Rust type that encodes the trie type in the Rust type.
#[inline]
pub fn as_typed_ref(
    &self,
) -> Typed<&FastCodePointTrie<'trie, T>, &SmallCodePointTrie<'trie, T>> {
    // SAFETY: `FastCodePointTrie` and `SmallCodePointTrie` are `repr(transparent)`
    // with `CodePointTrie`, so transmuting between the references is OK when the
    // actual trie type agrees with the semantics of the typed wrapper.
    match self.header.trie_type {
        TrieType::Fast => Typed::Fast(unsafe {
            core::mem::transmute::<&CodePointTrie<'trie, T>, &FastCodePointTrie<'trie, T>>(self)
        }),
        TrieType::Small => Typed::Small(unsafe {
            core::mem::transmute::<&CodePointTrie<'trie, T>, &SmallCodePointTrie<'trie, T>>(
                self,
            )
        }),
    }
}

Pattern:

#[repr(transparent)]
pub struct Pattern<B: PatternBackend> {
    _backend: PhantomData<B>,
    /// The encoded storage
    pub store: B::Store,
}
#[cfg(feature = "alloc")]
pub(crate) const fn from_boxed_store_unchecked(store: Box<B::Store>) -> Box<Self> {
    // Safety: Pattern is repr(transparent) over B::Store
    unsafe { core::mem::transmute(store) }
}

[Manishearth]

If adding the ~dozen additional zerovec call sites to this post would be important for your analysis,

No need, I was just looking for the general shapes, and some links to code (or inline code, that works too)

Seems like most of them are custom DSTs (which have always been somewhat annoying in Rust, though Rust does hope to improve that over time), and then there are a few one-offs like the ZeroTrie and GenricPatternItemULE ones.

I did consider the DST cases self evident, I just wasn't sure if there were more non-DST cases. I could look for them myself, but as I said you have more familiarity with this and I appreciate the work.

(There's a chance that a good solution will separate DSTs from non-DSTs)

Thanks! I will think about this more, I'm signing off now.

[sffc]

I put up #7610 as another attempt at this.