allow Allocators to be used as #[global_allocator]s#157153
Open
joboet wants to merge 3 commits into
Open
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
nia-e
reviewed
May 30, 2026
| /// * for wrappers of arbitrary allocators (which might end up being `Global`, | ||
| /// leading to infinite recursion). | ||
| #[unstable(feature = "allocator_api", issue = "32838")] | ||
| pub trait GlobalAllocator: Allocator {} |
Member
There was a problem hiding this comment.
Should this be an unsafe trait? If so the safety contract can just be that it doesn't use std, and so we can do what was suggested in rust-lang/libs-team#743
Member
|
Seems great! I just had |
nia-e
reviewed
May 30, 2026
| #[stable(feature = "global_alloc", since = "1.28.0")] | ||
| unsafe impl<A> GlobalAlloc for A | ||
| where | ||
| A: GlobalAllocator, |
Member
There was a problem hiding this comment.
Suggested change
| A: GlobalAllocator, | |
| A: GlobalAllocator + ?Sized, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The (hopefully) immanent stabilisation of the
Allocatortrait raises the question of what is to be done about the older, already-stableGlobalAlloctrait. In my opinion, having two nearly-identical traits for the same purpose is needlessly confusing. Going forward,Allocatoras the more modern interface should be the allocator trait.With
Allocatorbeing currently unstable, there is the possibility of implementingGlobalAllocfor allAllocators, thereby allowing them to be used as#[global_allocator]and allowing crates to seamlessly (and semver-compatibly) switch toAllocator. However, unconditionally implementingGlobalAllocpresents a footgun to users, as e.g. usingGlobalas#[global_allocator]will lead to infinite recursion. @nia-e initially tried to resolve this in e1b7097 (#156882) by using weird trait trickery to implementGlobalAllocfor every allocator exceptGlobal. But this does not go far enough, e.g. a bump allocator that itself allocates fromGlobalis similarly unsuitable as global allocator.Thus, with this PR, I'd like to propose adding a new marker trait for allocators that can be used as
#[global_allocator]:GlobalAlloccan then be implemented for allGlobalAllocators:This provides a backwards-compatible way for allocator libraries to switch to the new interface and allows deprecating
GlobalAlloc(not done here). Over time, I expect thatGlobalAllocwill become more and more of an implementation detail of the#[global_allocator]macro (for instance, one might add perma-unstable, hidden methods for things likegrow_zeroedthat are customised only by the blanket implementation).With regards to naming, I chose
GlobalAllocatorto mirrorAllocator.GlobalAllocshould probably be deprecated quickly after stabilisingGlobalAllocatorto avoid confusion. For the same reason, I think it'd be better to addGlobalAllocatorbefore stabilisingAllocator– but that is not a necessity.r? @nia-e
@rustbot label +I-libs-api-nominated