[WIP] Add support for zerocopy traits via a feature.

[mwanner]

I'm submitting a feature proposal.

Description

Allows uuid::Uuid to be read from or serialized to a byte slice without copying by using the zerocopy crate.

Motivation

I like the abstraction that zerocopy provides and UUIDs are often embedded in other structs I'd like to use zerocopy's traits on. Hiding this behind a feature that needs to be explicitly enabled should keep the general cost of this very low for the vast majority of users of uuid that do not use zerocopy.

Tests

Tests missing, therefore marked WIP.

Related Issue(s)

Not aware of any, possibly somewhat related to #472, though. Clearly adding another variant of as_bytes() via the zerocopy::AsBytes trait.

[KodrAus]

Thanks for the PR @mwanner! Since we’re looking to stabilize this crate I don’t think we can introduce any unstable public dependencies here. This seems like something that would be covered by the safer transmute API, but that’s still probably a ways off.

Do you know if zerocopy has any attributes you can apply to fields of types deriving one of its attributes to tweak how they’re implemented?

[mwanner]

Thanks for the PR @mwanner! Since we’re looking to stabilize this crate I don’t think we can introduce any unstable public dependencies here.

Would implementing the trait for Uuid as part of zerocopy itself be a better way forward? (Not entirely sure that's possible at all, but...)

This seems like something that would be covered by the safer transmute API, but that’s still probably a ways off.

Thanks for this link, I wasn't aware of this effort.

Do you know if zerocopy has any attributes you can apply to fields of types deriving one of its attributes to tweak how they’re implemented?

Not sure if that's what you're asking for, but there's e.g. U32<LittleEndian> providing means to read from and write to memory with necessary byte swapping. Clearly nothing fancier as would be required for different versions of UUIDs. However, it seems to me Uuid is internally represented as [u8; 16] and a const Uuid could in theory be created from a &[u8; 16] w/o any copying.

[KodrAus]

The Uuid::from_bytes and Uuid::as_bytes methods are both const, and on the master branch we do mark Uuid as #[repr(transparent)] so you should be able to safely transmute it from a [u8; 16], u128 etc, but only safely back into a [u8; 16].

I don't think we could create a const Uuid from a &[u8; 16] without copying, but we should be able to create a &Uuid from a &[u8; 16]. Is that what you were meaning?

[mwanner]

The Uuid::from_bytes and Uuid::as_bytes methods are both const, and on the master branch we do mark Uuid as #[repr(transparent)] so you should be able to safely transmute it from a [u8; 16], u128 etc, but only safely back into a [u8; 16].

I don't think we could create a const Uuid from a &[u8; 16] without copying, but we should be able to create a &Uuid from a &[u8; 16]. Is that what you were meaning?

Yes, sorry, that's what I meant. Is there currently a safe variant of the following thanks to repr(transparent)?

pub fn from_u8_array_ref(data: &[u8; 16]) -> &Uuid {
    unsafe { transmute(data) }
}

[KodrAus]

It looks like we don't currently have one, but we could add something like this in uuid:

impl Uuid {
    pub fn ref_from_bytes(data: &[u8; 16]) -> &Uuid;
}

and deal with the unsafety in there. I'm not sure whether it would be valid to cast other types just yet, but it could be possible to also do ref_from_u128 since Uuids alignment is looser than u128s?

[sivadeilra]

Hi, folks. I'd like to help drive this PR forward. I didn't think to check the PR list for uuid, and I ended up writing almost exactly the same PR as this one! :)

@KodrAus, could you help me understand this? "Since we’re looking to stabilize this crate I don’t think we can introduce any unstable public dependencies here." Is the requirement here that all dependencies of uuid also be stable, even if those dependencies are optional features?

Also, can you clarify what you mean by "stabilize"? My impression of the zerocopy crate is that it has essentially stabilized, but do you mean a more formal process involving the Rust Project itself?

Supporting zerocopy is really important for my team -- I am the dev lead for a team at Microsoft, working on Rust support in Windows. And we use a lot of GUIDs / UUIDs, of course. :) The workarounds listed above in this conversation don't really work for us, because we need to be able to embed UUIDs into other data structures, and those data structures all use zerocopy traits to declare that the entire structure can be safely transmuted.

[KodrAus]

Hey @sivadeilra 👋

The constraint we're working with is that any dependency that affects uuid's public API (so types in public functions, and trait implementations on public types) need to have a stable version (that is, 1.0+) otherwise we can't bump them without a corresponding major version bump to uuid, or a commitment to implementing all minor versions that come up before 1.0. I wouldn't want to appear to promise zero-copy support by including it now while uuid is unstable and then having to back it out later :)

We have actually since added a #[repr(transparent)] attribute to Uuid though so the best path forward for your code might be to create a newtype wrapper over Uuid that implements the necessary traits. I know it's not as nice as just having everything line up.

[sivadeilra]

Hi, thanks for responding.

We have actually since added a #[repr(transparent)] attribute [...]

Unfortunately, a newtype over Uuid won't work, because the zerocopy traits require that the inner type also implement the same traits, similar to how Send requires the same. And like Send, these traits are "unsafe" in the sense that they cannot be implemented in verifiable code, only derived using #[derive].

I've worked with the owner of zerocopy before on a few changes. My perception is that the crate is already considered "stable", and they already guarantee API stability, but obviously it's using pre-1.0 version numbers. If I contact the crate owner, and he agrees to raise the version number to 1.0 (and to meet the backward-compatibility stability guarantees), would that meet the requirements for adding this to Uuid?

[sivadeilra]

Ping, on my previous comment.

[KodrAus]

Hmm, I wouldn’t want to put pressure on the zero-copy maintainers to push a 1.0 before they’re ready to. I haven’t used the library myself so am not sure what prevents you from implementing the unsafe trait, so I’ll take a closer look at it.

[KodrAus]

Ah, I see, when it comes to implementing the traits the docs simply say “don’t” and they go to great effort to discourage you from implementing them directly. That’s a little unfortunate because it doesn’t give us many options to support it. Send isn’t really an equivalent comparison here, because even though it’s an auto-trait that’s only automatically implemented when all fields do you can still (and do need to) manually implement the unsafe trait yourself in order to build synchronisation primitives.

As it is we’re a bit stuck then because we can’t implement unstable traits in uuid, even when the API might be considered done, because Cargo will still produce compilation errors if two semver incompatible versions of that library end up in the same dependency tree.

[KodrAus]

If there’s no other option for you (such as using [u8; 16] and transmuting to Uuid on-demand, which we could offer a safe API for) we could consider calling this feature zerocopy-unstable and explicitly not consider bumps to the zerocopy version as semver breaking. If there’s a 0.7 release then we’ll end up pulling it in as a minor release and cause anybody depending on it to need to upgrade their own versions of zerocopy too. I’m a bit uncomfortable with that angle so we’d still need to re-evaluate things before we release 1.0.0 of uuid, but since zerocopy is quite niche and doesn’t give us another option we could consider that path.

Would that work for your needs?

[KodrAus]

Another option could be for zerocopy to instead add an optional uuid feature and implement their traits for it.

Since I’m currently working on a stable 1.0 of uuid that would mean we could put the trait implementation in the unstable library (zerocopy) instead of the stable one (uuid).

[sivadeilra]

I think the zerocopy-unstable option probably makes the most sense. It seems cleaner to me to put the implementations with the type, rather than in the crates that define the traits.

A bunch of us have been trying to push forward on supporting safe transmute traits directly in core, but there really hasn't been much momentum on that lately. I think traits like AsBytes really should be in core, since they define fundamental aspects of traits, and should eventually be auto-derived in the same way that Send and Sync are.

Would you accept the option of defining a feature for zerocopy-unstable, in the near term? If we ever get the safe-transmute story straightened out, then uuid could move to that, and we could remove the zerocopy-unstable feature or just make it a no-op.

[KodrAus]

@sivadeilra I think I’d prefer we define the impl in zerocopy once we’ve stabilized uuid if that’s at all possible. At least until zerocopy becomes stable then we could move it here. We can fall back to a zerocopy-unstable feature here if that’s not possible, but should probably also combine it with a direct RUSTFLAGS cfg so you have to opt-in to potential breakage more strongly.

It’s a bit of a shame just how disruptive semver bumps can be, even if there’s very little difference, but that’s the nature of it 🙂

[KodrAus]

I've opened a PR in #538 that adds unstable zerocopy support here. In order for it to work you need to add a RUSTFLAGS cfg, which makes it difficult for libraries to use, but applications can. If that's not suitable for your needs then I think adding support in zerocopy directly will be the way to go.

[sivadeilra]

I think that's a good place to start. I do appreciate the requirements for stability, and not casually taking dependencies on crates that might change.

Let's start with this, and I'll see if I can't nudge the safe-transmute process along. If that ever converges, then we can revisit this.

Thanks!

[KodrAus]

We’ve now added zerocopy support through #538