-
Notifications
You must be signed in to change notification settings - Fork 294
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
Add support for allocator-api2 #417
Conversation
Could the |
It does. However it can't be used when building for |
This doesn't work well due to the way Cargo features works. Consider this case:
Crate B will try to implement the allocator traits using allocator-api2, but this will fail because hashbrown will expect the standard library traits. I prefer making hashbrown uses std's Allocator trait only under the rustc-dep-of-std feature, and otherwise letting allocator-api2 decide which Allocator trait to use. |
Yes, if nothing would enable "nightly" feature in Originally I tried to do what you suggests. The problem with this is that "nightly" feature will enable "allocator-api2/nightly", enabling "allocator-api2" itself. |
Could we just make |
I found a problem with current approach. I tried few things to detect if "nightly" feature is enabled The possible workaround for this is to make |
Good point. In that case perhaps we should have 3 settings for hashbrown:
This forces crates which need the allocator API to opt-in to an implementation, while catching the case that we can't support at compile time (2 crates needing different allocator APIs). Since this is a breaking change, it will be part of hashbrown 0.14. I would also like the bumpalo support to be folded into this feature once fitzgen/bumpalo#201 is merged and published. |
I would like to avoid compilation error when two features are enabled. In last iteration I removed re-export of unstable API from I think the solution should be simple. |
nightly - unstable allocator_api allocator-api2 - stable version of allocator_api neither - custom trait implemented only for default allocator parameter
Ok. I think I figured it out. Typical crates would use Other crates would typically use If default features are disabled and neither "nightly" enables "allocator-api2/nightly" and then never uses it. One may wonder why. |
I removed custom |
Mention removal of that feature in changelog. Remove BumpWrapper and its test. Use another custom allocator in tests since bumpalo support for allocator-api2 is not released.
But only with nightly feature now
Thanks for working on this, I like this new setup! However I would like the feature names to be renamed:
This can be made to work by using a new feature of Cargo available since Rust 1.60: weak dependencies. Essentially the nightly line would become: nightly-allocator = ["allocator-api2?/nightly", "nightly", "bumpalo?/allocator_api"] This will disable the behavior of automatically enabling the optional dependency when the |
Oh, great! I somehow missed the feature that I've being waiting for so long :) |
Avoid multiplying flavors of "nightly" feature
default = ["ahash", "inline-more"] | ||
default = ["ahash", "inline-more", "allocator-api2"] | ||
|
||
nightly = ["allocator-api2?/nightly", "bumpalo/allocator_api"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nightly = ["allocator-api2?/nightly", "bumpalo/allocator_api"] | |
nightly = ["allocator-api2?/nightly", "bumpalo?/allocator_api"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bumpalo
is not optional
dependency, it's dev-dependency
.
So ?
is not applicable and not necessary, nightly
won't add bumpalo
dependency except for examples, tests and benches.
@bors r+ |
☀️ Test successful - checks-actions |
|
This change allows using hashbrown with other crates that support
allocator-api2
.It opens possibility for any allocator that supports
allocator-api2
to be used withhashbrown
.allocator-api2
mimics types and functions of unstableallocator-api
feature but works on stable.hashbrown
will be usingallocator-api2
only when `"nightly" feature is not enabled.Typical use-case for
allocator-api2
is to use it both with and without"nightly" feature, however
hashbrownis special-case since it is built as part of
stdand
allocator-api2` is not ready for this.If fitzgen/bumpalo#201 is accepted, explicit support for
bumpalo
would be obsolete.caveat: Either
"nightly"
,"allocator-api2"
or both features should be enabled forhashbrown
to compile.