Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try add covariance for containers #170

Open
zetanumbers opened this issue Jun 27, 2022 · 3 comments
Open

Try add covariance for containers #170

zetanumbers opened this issue Jun 27, 2022 · 3 comments

Comments

@zetanumbers
Copy link
Contributor

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.

@fitzgen
Copy link
Owner

fitzgen commented Jun 27, 2022

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.

@zetanumbers
Copy link
Contributor Author

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>

@douglas-raillard-arm
Copy link

douglas-raillard-arm commented Apr 11, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants