Use allocator-api through allocator-api2 crate #201

zakarumych · 2023-04-05T08:49:21Z

This change allows to use mirror of the allocator-api on stable channel.
Types imported from allocator-api2 are reexports from core and alloc crates when "nightly" feature is enabled.
Otherwise allocator-api2 defines types to mimic unstable core and alloc types and functions, stable stuff is re-exported.

The goal is to add support for allocator-api2 into as many crates as possible.
The bumpalo crate is first on the list.

zakarumych · 2023-04-05T09:26:03Z

Apparently this would require bumping MSRV to 1.63

fitzgen

Thanks for the PR!

Can we make the dependency optional? Right now, bumpalo doesn't have any mandatory dependencies, and I'd like to keep it that way.

Additionally, once the dependency is optional, we should have a blurb in the README just before/after the existing blurb for the allocator_api feature about what enabling it does.

fitzgen · 2023-04-10T19:47:02Z

src/lib.rs

-            .map(|p| NonNull::slice_from_raw_parts(p, new_layout.size()))
+            .map(|p| unsafe {
+                NonNull::new_unchecked(ptr::slice_from_raw_parts_mut(p.as_ptr(), new_layout.size()))
+            })


Why these changes? Seems unnecessary?

To remove all unstable rust features. Except that I had to put one back, so this one may stay as well I guess

zakarumych · 2023-04-11T08:46:12Z

How do you suggest enabling "nightly" in allocator-api2?

zakarumych · 2023-04-11T12:17:43Z

I found a problem with the approach in allocator-api2 - replacing stable mirror API with unstable API re-export with a feature makes all dependent crates manually enable unstable feature(allocator_api).
If some other crate would enable "allocator-api2/nightly" feature and not enable "bumpalo/nightly" then bumpalo won't compile.

So I changed it.
allocator-api2 is now always stable mirror and crates choose from using allocator-api2 and alloc crate, picking alloc when they enable nightly Rust feature allocator_api.

zakarumych · 2023-04-11T12:19:15Z

I tried to forward "nightly" feature back from allocator-api2 into bumpalo, but it requires other unstable features.

fitzgen

Aside: it seems like it would make sense for allocator_api2 to have a cargo feature that, when enabled, made the crate re-export the std types/traits rather than polyfilling them. I don't think bumpalo woulds use that feature, but it would help avoid ecosystem splitting and annoying build errors where some crates are using the actual Allocator and others are using allocator_api2::Allocator.

fitzgen · 2023-04-21T21:00:48Z

src/lib.rs

-#![cfg_attr(feature = "allocator_api", feature(allocator_api))]
+#![cfg_attr(feature = "nightly", feature(allocator_api))]


Ideally, the API and cargo features for enabling the nightly allocator API support in bumpalo wouldn't change just because we can alternatively use the allocator_api2 crate, so the "nightly" feature should keep the "allocator_api" name.

fitzgen · 2023-04-21T21:03:19Z

src/lib.rs

@@ -1886,6 +1891,7 @@ unsafe impl<'a> alloc::Alloc for &'a Bump {
    }
 }

+#[cfg(feature = "allocator_api")]


This should be

#[cfg(any(feature = "allocator_api", feature = "allocator_api2"))]

zakarumych · 2023-04-22T06:26:21Z

It was made like this originally.

However this doesn't work since user would have to enable unstable #![feature(allocator_api)] if any crate enables "nightly" in allocator-api2.

I found no way to enable #![feature(allocator_api)] conditionally based on allocator-api2 features.
Tried using macro, but inner macro attributes are unstable.
It's also impossible to get custom cfg flag from dependency.

I tried different route and make blanket impls of allocator_api2::Allocator for core::alloc::Allocator types.
This way allocators that implement core::alloc::Allocator would be compatible.
Allocators that use allocator-api2 would implement core::alloc::Allocator with "nightly" feature as well.

Unfortunately I got stuck on conflicting impls for references. Tried using specialization feature to resolve the conflict, but with no luck.

So currently PR is blocked on resolving this issue. I'll return to it later when I have free time again.

zakarumych · 2023-05-01T16:19:49Z

Ok. I think it should be consistent and usable now.
allocator-api2 re-exports core, alloc and std with "nightly" feature.
Other crates that use it should have means to enable #![feature(allocator_api)] to avoid compilation errors on nightly channel.
bumpalo does. Even if direct dependent crates won't enable it since it doesn't care about nightly, indirect dependent crates would be able to enable it if they encounter compilation error when some other crate in the tree enable "allocator-api2/nightly".

fitzgen

Looks great, thanks for your patience as we worked out the exact kinks of how the cargo features and cfgs would work!

fitzgen · 2023-05-09T17:47:56Z

It does look like this needs a little rebase to deal with CHANGELOG.md conflicts, and in that case can you also remove the now-obsolete bits in the changelog about the "nightly" feature? I was going to do that myself, but since it needs a rebase anyways...

"nightly" enables use of unstable allocator_api. "allocator_api" without "nightly" enables usage of allocator-api2

Keep feature semantics

zakarumych · 2023-05-10T10:26:42Z

It's ready for merge now

fitzgen

Thanks!!

Add support for allocator-api2 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 with `hashbrown`. `allocator-api2` mimics types and functions of unstable `allocator-api` feature but works on stable. `hashbrown` will be using `allocator-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 `hashbrown` is special-case since it is built as part of `std` and `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 for `hashbrown` to compile.

zakarumych · 2023-05-22T15:44:33Z

@fitzgen can you provide ETA for next release with this?

fitzgen · 2023-05-22T21:46:49Z

Published!

zakarumych mentioned this pull request Apr 5, 2023

Add support for allocator-api2 rust-lang/hashbrown#417

Merged

fitzgen reviewed Apr 10, 2023

View reviewed changes

fitzgen reviewed Apr 21, 2023

View reviewed changes

fitzgen approved these changes May 9, 2023

View reviewed changes

zakarumych and others added 6 commits May 10, 2023 11:31

Use allocator-api through allocator-api2 crate

98f86e5

Gently bump MSRV to 1.63

a95e93e

No need for std in allocator-api2

073d13e

Use either allocator_api or allocator-api2

f04be75

"nightly" enables use of unstable allocator_api. "allocator_api" without "nightly" enables usage of allocator-api2

Use allocator-api2 only on stable

edd4f9d

Keep feature semantics

Rebase and update changelog

81f5b92

zakarumych force-pushed the main branch from 64dc1fe to 81f5b92 Compare May 10, 2023 08:40

fitzgen approved these changes May 11, 2023

View reviewed changes

fitzgen merged commit cfe944f into fitzgen:main May 11, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use allocator-api through allocator-api2 crate #201

Use allocator-api through allocator-api2 crate #201

zakarumych commented Apr 5, 2023 •

edited

zakarumych commented Apr 5, 2023

fitzgen left a comment

fitzgen Apr 10, 2023

zakarumych Apr 11, 2023

zakarumych commented Apr 11, 2023

zakarumych commented Apr 11, 2023 •

edited

zakarumych commented Apr 11, 2023

fitzgen left a comment

fitzgen Apr 21, 2023

fitzgen Apr 21, 2023

zakarumych commented Apr 22, 2023

zakarumych commented May 1, 2023

fitzgen left a comment

fitzgen commented May 9, 2023

zakarumych commented May 10, 2023

fitzgen left a comment

zakarumych commented May 22, 2023

fitzgen commented May 22, 2023

		#![cfg_attr(feature = "allocator_api", feature(allocator_api))]
		#![cfg_attr(feature = "nightly", feature(allocator_api))]

Use allocator-api through allocator-api2 crate #201

Use allocator-api through allocator-api2 crate #201

Conversation

zakarumych commented Apr 5, 2023 • edited

zakarumych commented Apr 5, 2023

fitzgen left a comment

Choose a reason for hiding this comment

fitzgen Apr 10, 2023

Choose a reason for hiding this comment

zakarumych Apr 11, 2023

Choose a reason for hiding this comment

zakarumych commented Apr 11, 2023

zakarumych commented Apr 11, 2023 • edited

zakarumych commented Apr 11, 2023

fitzgen left a comment

Choose a reason for hiding this comment

fitzgen Apr 21, 2023

Choose a reason for hiding this comment

fitzgen Apr 21, 2023

Choose a reason for hiding this comment

zakarumych commented Apr 22, 2023

zakarumych commented May 1, 2023

fitzgen left a comment

Choose a reason for hiding this comment

fitzgen commented May 9, 2023

zakarumych commented May 10, 2023

fitzgen left a comment

Choose a reason for hiding this comment

zakarumych commented May 22, 2023

fitzgen commented May 22, 2023

zakarumych commented Apr 5, 2023 •

edited

zakarumych commented Apr 11, 2023 •

edited