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

Feature request: make EcoVec::new() const #27

Closed
Kmeakin opened this issue Jul 31, 2023 · 8 comments
Closed

Feature request: make EcoVec::new() const #27

Kmeakin opened this issue Jul 31, 2023 · 8 comments

Comments

@Kmeakin
Copy link
Contributor

Kmeakin commented Jul 31, 2023

It would be nice if EcoVec::new() were callable in const contexts, to match Vec::new()

@laurmaedje
Copy link
Member

laurmaedje commented Jul 31, 2023

Unfortunately, it is not possible in the current design because EcoVec::new depends on a static and const fns can't reference statics.

Vec initializes its pointer with NonNull::dangling and uses its capacity to track whether it is allocated. EcoVec in comparison initializes its pointer with the address of a static SENTINEL value and uses that to track whether it is allocated. Using NonNull::dangling isn't possible because we have no other means to track whether we are allocated and NonNull::dangling could be a valid pointer to an allocation.

Null cannot be used in either case because a slice's pointer can't be null and we want deref to slice to be a no-op (that's a design goal).

@laurmaedje laurmaedje closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2023
@Kmeakin
Copy link
Contributor Author

Kmeakin commented Jul 31, 2023

Could the sign bit in len be used as the allocated flag? Allocations of more than isize::MAX are not allowed in Rust, so self.len > (isize::MAX as usize) could be used to signal an unallocated EcoVec

@laurmaedje
Copy link
Member

Then EcoVec<T>'s deref to [T] isn't a no-op once again. Moreover, EcoString (which uses EcoVec internally) uses that bit to distinguish between inline and heap (EcoVec<u8>) representation.

@Kmeakin
Copy link
Contributor Author

Kmeakin commented Aug 1, 2023

Possible implementation: #28

@laurmaedje laurmaedje reopened this Aug 1, 2023
@laurmaedje
Copy link
Member

Implemented in #28.

I'll publish a new release soon.

@flyingmutant
Copy link

Sorry for the ping, any news about the release?

@laurmaedje
Copy link
Member

@flyingmutant I'll make one later today.

@laurmaedje
Copy link
Member

@flyingmutant I've just released ecow 0.1.2.

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