-
Notifications
You must be signed in to change notification settings - Fork 142
Description
Overview
Make zerocopy-derive an optional dependency in order to significantly improve compile times for those who don't need the derives.
Motivation
Compile times are important, and are a major blocker for zerocopy adoption, especially for high-profile ecosystem crates who are very strict with their compile time budgets.
Benchmarking suggests that 85-90% of zerocopy's build time is due to zerocopy-derive. The most recent numbers from the current implementation imply that it's closer to 95%.
If we can make zerocopy-derive an optional dependency, then the --no-default-features configuration will have zero dependencies and will take less than 1/3 of a second to build on some platforms.
Design
There are two reasons that zerocopy depends on zerocopy-derive:
- zerocopy re-exports the derives for convenience
- zerocopy uses the derives for its own types
The first of these is a non-issue - users disabling the zerocopy-derive dependency are presumably not making use of the derives anyway.
The second presents a bigger problem. The derives don't just save boilerplate, they also perform important soundness verification that we'd otherwise have to reason through ourselves. It would be a big step backwards in terms of confidence in the correctness of our code to get rid of the derives entirely and revert to manual implementations.
A first stab at this might be to feature-gate our derives - in other words, use them when the derive feature is enabled, and use manual impls when it's disabled. This would in theory ensure that, so long as we test with the derive feature in CI, the manual impls can never be unsound.
However, that's not quite enough. The soundness of a trait impl isn't just based on its presence or absence. It's also based on the trait bounds placed on that impl. Consider:
#[repr(transparent)]
struct Wrapper<T>(T);
unsafe impl<T> FromBytes for Wrapper<T> {}In this example, the FromBytes impl is unsound because T is unbound. The impl is only sound if we add a T: FromBytes bound (which is exactly what the custom derive does).
So we not only need to verify that some impl is sound, but that the impls we emit - with their particular trait bounds - are sound. In order to solve this problem, we introduce an impl_or_verify! macro:
macro_rules! impl_or_verify {
// Implement `$trait` for `$ty` with no bounds.
($ty:ty: $trait:ty) => {
impl_or_verify!(@impl { unsafe_impl!($ty: $trait); });
impl_or_verify!(@verify $trait, { impl Subtrait for $ty {} });
};
// Implement all `$traits` for `$ty` with no bounds.
($ty:ty: $($traits:ty),*) => {
$( foobar!($ty: $traits); )*
};
// For all `$tyvar` with no bounds, implement `$trait` for `$ty`.
($tyvar:ident => $trait:ident for $ty:ty) => {
impl_or_verify!(@impl { unsafe_impl!($tyvar => $trait for $ty); });
impl_or_verify!(@verify $trait, { impl<$tyvar> Subtrait for $ty {} });
};
// For all `$tyvar: ?Sized` with no bounds, implement `$trait` for `$ty`.
($tyvar:ident: ?Sized => $trait:ident for $ty:ty) => {
impl_or_verify!(@impl { unsafe_impl!($tyvar: ?Sized => $trait for $ty); });
impl_or_verify!(@verify $trait, { impl<$tyvar: ?Sized> Subtrait for $ty {} });
};
// For all `$tyvar: $bound`, implement `$trait` for `$ty`.
($tyvar:ident: $bound:path => $trait:ident for $ty:ty) => {
impl_or_verify!(@impl { unsafe_impl!($tyvar: $bound => $trait for $ty); });
impl_or_verify!(@verify $trait, { impl<$tyvar: $bound> Subtrait for $ty {} });
};
// For all `$tyvar: $bound + ?Sized`, implement `$trait` for `$ty`.
($tyvar:ident: ?Sized + $bound:path => $trait:ident for $ty:ty) => {
impl_or_verify!(@impl { unsafe_impl!($tyvar: ?Sized + $bound => $trait for $ty); });
impl_or_verify!(@verify $trait, { impl<$tyvar: ?Sized + $bound> Subtrait for $ty {} });
};
// For all `$tyvar: $bound` and for all `const $constvar: $constty`,
// implement `$trait` for `$ty`.
($tyvar:ident: $bound:path, const $constvar:ident: $constty:ty => $trait:ident for $ty:ty) => {
impl_or_verify!(@impl { unsafe_impl!($tyvar: $bound, const $constvar: $constty => $trait for $ty); });
impl_or_verify!(@verify $trait, { impl<$tyvar: $bound, const $constvar: $constty> Subtrait for $ty {} });
};
(@impl $impl_block:tt) => {
#[cfg(not(any(feature = "derive", test)))]
const _: () = { $impl_block };
};
(@verify $trait:ident, $impl_block:tt) => {
#[cfg(any(feature = "derive", test))]
const _: () = {
trait Subtrait: $trait {}
$impl_block
};
};
}When the derive feature is disabled, impl_or_verify! simply emits the impl. When the derive feature is enabled, though, it emits code which only compiles if the manual impl's trait bounds are at least as restrictive as those emitted by the derive. This effectively ties our manual impls to the custom derive, ensuring that the manual impls are sound so long as the custom derive is sound.
There's one final problem, however. There's no way to actually guarantee that the impl_or_verify! macro is ever used in "derive" mode. Consider:
#[cfg_attr(feature = "derive", derive(FromBytes))]
#[repr(transparent)]
struct Wrapper<T>(T);
#[cfg(not(feature = "derive"))]
impl_or_verify!(T => FromBytes for Wrapper<T>);In this case, the unsound manual impl would never be detected because the impl_or_verify! macro would never be compiled with the derive feature enabled. In order to hedge against this (albeit slight) risk, we opt to still require a safety comment for uses of impl_or_verify!. That is, when combined with our use of deny(clippy::undocumented_unsafe_blocks), a call to impl_or_verify! will only compile when combined with safety_comment!:
safety_comment! {
/// SAFETY:
/// `Wrapper<T>` has the same layout as `T`, so `T: FromBytes` implies that `Wrapper<T>: FromBytes`.
impl_or_verify!(T: FromBytes => FromBytes for Wrapper<T>);
}This serves both to sanity-check our impl in case it's only compiled when not(feature = "derive") and also serves to document the reasoning behind a set of trait bounds, which is useful on its own independent of the not(feature = "derive") issue.