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

Hashbrown fails to compile if 0.14.x and 0.15.x are in the same project, only one with the nightly feature #564

Open
orlp opened this issue Oct 3, 2024 · 9 comments

Comments

@orlp
Copy link

orlp commented Oct 3, 2024

If a project (most likely indirectly through dependencies) depends on both version 0.14.x and 0.15.x of hashbrown, and only one of the two dependencies has the nightly feature flag set, the project will fail to compile due to using the allocator_api without enabling the unstable feature for it:

error[E0658]: use of unstable library feature 'allocator_api'
    --> /Users/orlp/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.14.5/src/raw/mod.rs:4329:23
     |
4329 |                 alloc.deallocate(ptr, layout);
     |                       ^^^^^^^^^^
     |
     = note: see issue #32838 <https://github.com/rust-lang/rust/issues/32838> for more information
     = help: add `#![feature(allocator_api)]` to the crate attributes to enable
     = note: this compiler was built on 2024-10-02; consider upgrading it if it is out of date

This can be reproduced by making a clean new project and adding the following two lines to Cargo.toml:

hashbrown_14 = { package = "hashbrown", version = "=0.14.5", features = [] }
hashbrown_15 = { package = "hashbrown", version = "=0.15.0", features = ["nightly"] }

If it's just one of the two the project compiles fine, regardless of the nightly feature flag. If neither or both have the nightly feature flag everything compiles fine as well. However if one has the nightly feature flag and the other does not, the project doesn't compile.

@orlp orlp changed the title Hashbrown fails to compile if 0.14.x and 0.15.x are in the same project, one with nightly feature Hashbrown fails to compile if 0.14.x and 0.15.x are in the same project, only one with the nightly feature Oct 3, 2024
@orlp
Copy link
Author

orlp commented Oct 3, 2024

The problem appears to be that allocator-api2's recommended usage is inherently flawed:

Your library MAY provide a feature that will enable "allocator-api2/nightly". When this feature is enabled, your library MUST enable unstable #![feature(allocator_api)] or it may not compile. If feature is not provided, your library may not be compatible with the rest of the users and cause compilation errors on nightly channel when some other crate enables "allocator-api2/nightly" feature.

Feature unification makes the requirement "When this feature is enabled your library MUST enable unstable #![feature(allocator_api)] or it may not compile" fundamentally impossible to fulfill.

@Amanieu
Copy link
Member

Amanieu commented Oct 3, 2024

I'm not sure there's anything actionable we can do on hashbrown's side...

@cuviper
Copy link
Member

cuviper commented Oct 3, 2024

allocator-api2/nightly re-exports from alloc, but hashbrown/nightly is already using alloc directly -- so I think there's no reason to forward the nightly feature at all. That will mean its Allocator won't match that from allocator-api2 (without nightly), but really, it's already a breaking change that these features change the types in public API.

vmagro added a commit to facebookincubator/antlir that referenced this issue Oct 11, 2024
Summary:
Use nightly Rust compiler and enable the `nightly` feature of `hashbrown` so
that it compiles (see rust-lang/hashbrown#564)

This wouldn't really be a problem if we correctly copied the internally used
versions of Rust libraries, but we miss crate versions in most cases so
sometimes skew causes problems like this.

Disable broken sendstream test

Fix #219

Test Plan: Export to PR

Differential Revision: D64256300
vmagro added a commit to facebookincubator/antlir that referenced this issue Oct 11, 2024
Summary:
Pull Request resolved: #264

Use nightly Rust compiler and enable the `nightly` feature of `hashbrown` so
that it compiles (see rust-lang/hashbrown#564)

This wouldn't really be a problem if we correctly copied the internally used
versions of Rust libraries, but we miss crate versions in most cases so
sometimes skew causes problems like this.

Disable broken sendstream test

Fix #219

Test Plan: Export to PR

Differential Revision: D64256300
vmagro added a commit to facebookincubator/antlir that referenced this issue Oct 11, 2024
Summary:
Pull Request resolved: #264

Use nightly Rust compiler and enable the `nightly` feature of `hashbrown` so
that it compiles (see rust-lang/hashbrown#564)

This wouldn't really be a problem if we correctly copied the internally used
versions of Rust libraries, but we miss crate versions in most cases so
sometimes skew causes problems like this.

Disable broken sendstream test

Fix #219

Test Plan: Export to PR

Differential Revision: D64256300
vmagro added a commit to facebookincubator/antlir that referenced this issue Oct 11, 2024
Summary:
Pull Request resolved: #264

Use nightly Rust compiler and enable the `nightly` feature of `hashbrown` so
that it compiles (see rust-lang/hashbrown#564)

This wouldn't really be a problem if we correctly copied the internally used
versions of Rust libraries, but we miss crate versions in most cases so
sometimes skew causes problems like this.

Disable broken sendstream test

Fix #219

Test Plan: Export to PR

Differential Revision: D64256300
facebook-github-bot pushed a commit to facebookincubator/antlir that referenced this issue Oct 11, 2024
Summary:
Pull Request resolved: #264

Use nightly Rust compiler and enable the `nightly` feature of `hashbrown` so
that it compiles (see rust-lang/hashbrown#564)

This wouldn't really be a problem if we correctly copied the internally used
versions of Rust libraries, but we miss crate versions in most cases so
sometimes skew causes problems like this.

Disable broken sendstream test

Fix #219

Test Plan: Export to PR

Reviewed By: justintrudell

Differential Revision: D64256300

fbshipit-source-id: 2d9965e3065dd9e8632a65b606d2b1210014777c
@BlinkyStitt
Copy link

I just ran into this because of dependencies of my dependencies using hasbrown 14 but my code having 15. I think the best action for hashbrown here is to document this so it comes up in search easily. The error message was confusing but thankfully searching the issue tracker for "allocator_api" brought me here.

@sassman
Copy link

sassman commented Dec 13, 2024

seems to be the same or similar as #483

@Imberflur
Copy link

Hi, we just ran into this when updating dependencies and can't work around it because regalloc2 depends on allocator-api2 without any feature to enable #![feature(allocator_api)] in regalloc2 itself. bytecodealliance/regalloc2#209

We aren't relying on the allocator api aspect of hashbrown/nightly (afaik) but the other parts of this feature are very useful.

Would this proposed solution be acceptable?

allocator-api2/nightly re-exports from alloc, but hashbrown/nightly is already using alloc directly -- so I think there's no reason to forward the nightly feature at all. That will mean its Allocator won't match that from allocator-api2 (without nightly), but really, it's already a breaking change that these features change the types in public API.

Another approach that I think would work for us is if hashbrown exposed a separate feature that enabled everything from the nightly feature except the allocator api related items.

@NeuralModder
Copy link

NeuralModder commented Jan 24, 2025

(I come from the same project as Imbris/Imberflur)

I third the suggestion from cuviper, in the case of this specific crate it should work. However, if the allocator-api2/nightly feature is activated by another dependency of a project using hashbrown, it must be noted that it would no longer be possible to compile the project without activating hashbrown/nightly, which I suppose is acceptable.

Really, it is (and should be)(mostly) pointless/redundant to have both allocator-api2 and nightly features enabled, since the allocator_api2 crate is literally just a replacement for the unstable allocator-api. Though some tests in src/map.rs and src/raw/mod.rs use the allocator_api2 crate directly, so I presume they've never worked without the allocator-api2 feature, and will use normal allocator_api2 even when nightly is enabled if allocator-api2/nightly is no longer enabled by the nightly feature.

EDIT: Can confirm, in our case the fix is literally as simple as removing the allocator-api2/nightly feature:

diff --git a/Cargo.toml b/Cargo.toml
index 7bc1167..1d28792 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -47,7 +47,7 @@ default = ["default-hasher", "inline-more", "allocator-api2", "equivalent", "raw

 # Enables use of nightly features. This is only guaranteed to work on the latest
 # version of nightly Rust.
-nightly = ["allocator-api2?/nightly", "bumpalo/allocator_api"]
+nightly = ["bumpalo/allocator_api"]

 # Enables the RustcEntry API used to provide the standard library's Entry API.
 rustc-internal-api = []

I don't think this change should break anything, so it could be added in a patch release.

github-actions bot pushed a commit to veloren/veloren that referenced this issue Jan 26, 2025
Tried to run `cargo update`, but that broke things, see
rust-lang/hashbrown#564.

Updated:
- itertools to 0.14
- kiddo to 5
- petgraph to 0.7
- notify to 8
- wasmtime and wasmtime-wasi to 29
  Note: this causes even bigger problems if one were to try to run
  `cargo update` since even if one were to add certain transitive
  dependencies to common/Cargo.toml and enable the necessary feature
  flags, a transitive dependency here also depends on allocator-api2 but
  without exposing the proper feature flag. A fix is underway, see
  bytecodealliance/regalloc2#209
- axum to 0.8
- discord-sdk to 0.4
- minifb to 0.28
@Amanieu
Copy link
Member

Amanieu commented Jan 26, 2025

Yes, I think that would work. There is a small probability of breakage if someone was relying on hashbrown's nightly feature to enable allocator-api2/nightly, but that can be easily fixed by manually enabling allocator-api2/nightly.

Arguably the nightly feature should probably be deprecated and removed from allocator-api2 since there isn't really a good way to use it, but that discussion is already under way in zakarumych/allocator-api2#19. IMO allocator-api2 should only become a wrapper around the standard library API once that becomes stable and doesn't require #![feature] to use.

NeuralModder pushed a commit to NeuralModder/hashbrown that referenced this issue Jan 27, 2025
See rust-lang#564 for rationale. Tentatively fixes said issue and rust-lang#483, though
the root cause is up to the `allocator-api2` crate to fix.
@NeuralModder
Copy link

NeuralModder commented Jan 27, 2025

There is a small probability of breakage if someone was relying on hashbrown's nightly feature to enable allocator-api2/nightly, but that can be easily fixed by manually enabling allocator-api2/nightly.

Considering how the stable part of allocator-api2 is, by my understanding, meant as a drop-in replacement for the nightly api, and all issues we've seen so far have stemmed from allocator-api2/nightly being enabled when unexpected instead of the other way around, I find breakage highly unlikely.

Either way, I've started a PR for this change: #606; it turns out the direct use of allocator-api2 in the tests was indeed problematic, though the fix was thankfully easy.

NeuralModder pushed a commit to NeuralModder/hashbrown that referenced this issue Jan 27, 2025
See rust-lang#564 for rationale. Tentatively fixes said issue and rust-lang#483, though
the root cause is up to the `allocator-api2` crate to fix.
Direct use of `allocator-api2` in tests had to be replaced with the
indirection used elsewhere in the crate.
NeuralModder pushed a commit to NeuralModder/hashbrown that referenced this issue Jan 27, 2025
See rust-lang#564 for rationale. Tentatively fixes said issue and rust-lang#483, though
the root cause is up to the `allocator-api2` crate to fix.
Direct use of `allocator-api2` in tests had to be replaced with the
indirection used elsewhere in the crate.
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

7 participants