bumpalo icon indicating copy to clipboard operation
bumpalo copied to clipboard

Try add covariance for containers

Open zetanumbers opened this issue 2 years ago • 3 comments

Enables a small(?) but valid set of cases. May come with a small penalty, until we have Unique built-in. Tho it seems like even in std Unique<T> does not enable appropriate optimizations currently, based on my experiments with std::vec::Vec. On the other hand Box enables "ownership optimizations", maybe due to #[lang = "..."] attribute.

Make sure to benchmark difference.

zetanumbers avatar Jun 27 '22 19:06 zetanumbers

I doubt that lifetime variance has any effect on optimizations. AFAIK that's all erased by the time code goes to LLVM, and I'd expect that any rustc optimizations for Box are looking at the lang attributes.

fitzgen avatar Jun 27 '22 21:06 fitzgen

I doubt that lifetime variance has any effect on optimizations.

Of course, it's just that there's currently no nonaliased covariant pointer for rust, so the only option is maybe aliased covariant NonNull<T>, vs. nonaliased but invariant &mut ManuallyDrop<T>

zetanumbers avatar Jun 27 '22 21:06 zetanumbers

I just bumped into this issue while replacing a std::boxed::Box inside a recursive enum like this one:

struct Value<'a> {
    Str(&'a str),
    Wrapped(Box<Value<'a>>)
} 

The consuming code rightfully expects Value<'a> to be covariant in 'a, but that breaks with bumpalo currently. For example the code relies in multiple places on Value<'static> to always be a valid value for a Value<'a> parameter.

EDIT: Is there any workaround for that issue ? EDIT2: So I ended up with turning the Box into std::ptr::NonNull and turning it back into a Box when required.

douglas-raillard-arm avatar Apr 11 '23 18:04 douglas-raillard-arm