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

Rename AllocRef to Allocator and alloc to allocate #76

Closed
TimDiekmann opened this issue Nov 15, 2020 · 3 comments · Fixed by rust-lang/rust#79286
Closed

Rename AllocRef to Allocator and alloc to allocate #76

TimDiekmann opened this issue Nov 15, 2020 · 3 comments · Fixed by rust-lang/rust#79286
Labels
A-Alloc Trait Issues regarding the Alloc trait Proposal

Comments

@TimDiekmann
Copy link
Member

With rust-lang/rust#77187 we released AllocRef to the wild. It was pointed out, the names AllocRef and alloc are confusing sometimes. The main reason for this, is, that "alloc" is used as a verb and as a noun.

in rust-lang/rust#77187 (comment) @Aaron1011 noted, that:

[Box::alloc_ref] sounds like we're allocating a reference, not obtaining a reference to the allocator.

However, alloc_ref is the lowercased name of the Trait. @Amanieu also suggested, to rename it to Box::allocator, but the term allocator is never used anywhere, so I decided to stick with alloc_ref for now.

I think, it makes sense, to rename AllocRef to Allocator and (de)alloc to (de)allocate. The main reason for calling it AllocRef instead of Alloc was, that we wanted to express, that AllocRef should be a ZST or a reference to the actual allocator (not a reference to an "alloc" 😉 ). This however is also pointed out in the documentation and we don't expect many people to implement an allocator.
A very minor downside is the longer name of Allocator and allocate, but it's way more clear, if you read those terms. The trait will also not be used often directly. In most cases, it will simply be passed to Box or Vec. So most people neither have to call allocate or deallocate, nor have to import Allocator into scope. They probably won't even care, how the allocator is implemented (ZST or reference) as long they can simply pass an allocator to those structs.

This proposal also includes renaming Box::alloc_ref to Box::allocator.

cc @Amanieu @Lokathor @Wodann @CAD97 @scottjmaddox @vertexclique

@Wodann
Copy link

Wodann commented Dec 4, 2020

When reviewing the PR, I couldn't help but wonder whether people had similar confusions about whether AllocError is an allocation error or an error type for allocators.

I know this goes beyond the scope of the original proposal, but since we are asking ourselves the question; I'm wondering whether there are more instances that might be ambiguous?

@TimDiekmann
Copy link
Member Author

Hmm, isn't AllocError both an allocation error and the error type for allocators? 😄
The more I play around with the Allocator trait, the more I think, that the error should be associative but I leave this question open (and #23 closed) until I finished some work with the storage-approach for collections but the concerns are very valid IMO.

I know this goes beyond the scope of the original proposal, but since we are asking ourselves the question; I'm wondering whether there are more instances that might be ambiguous?

Do you have something in mind specifically?

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 4, 2020
…or,Wodann,m-ou-se

Rename `AllocRef` to `Allocator` and `(de)alloc` to `(de)allocate`

Calling `Box::alloc_ref` and `Vec::alloc_ref` sounds like allocating a reference. To solve this ambiguity, this renames `AllocRef` to `Allocator` and `alloc` to `allocate`. For a more detailed explaination see rust-lang/wg-allocators#76.

closes rust-lang/wg-allocators#76

r? `@KodrAus`
`@rustbot` modify labels: +A-allocators +T-libs
`@rustbot` ping wg-allocators
@Wodann
Copy link

Wodann commented Dec 5, 2020

I think I'm too close to the subject matter to be confused by it, but I think @Aaron1011's original point was very valid. So maybe they can speak from experience?

@TimDiekmann TimDiekmann mentioned this issue Dec 7, 2020
18 tasks
berkus added a commit to metta-systems/vesper that referenced this issue Dec 29, 2020
morimolymoly pushed a commit to morimolymoly/r-efi-alloc that referenced this issue Feb 22, 2022
dvdhrm pushed a commit to r-efi/r-efi-alloc that referenced this issue May 5, 2022
Needed since rust-lang/wg-allocators#76

Signed-off-by: Mizuho MORI <[email protected]>
(drop Cargo.lock-changes from commit)
Signed-off-by: David Rheinsberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Alloc Trait Issues regarding the Alloc trait Proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants