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

GlobalAlloc::realloc’s parameters are backward #3

Closed
SimonSapin opened this issue May 3, 2019 · 5 comments
Closed

GlobalAlloc::realloc’s parameters are backward #3

SimonSapin opened this issue May 3, 2019 · 5 comments
Labels
A-Alloc Trait Issues regarding the Alloc trait Discussion Issues containing a specific idea, which needs to be discussed in the WG

Comments

@SimonSapin
Copy link
Contributor

SimonSapin commented May 3, 2019

The GlobalAlloc trait is stable with this method:

unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8

(And similarly the std::alloc::realloc function.)

Note new_size: usize instead of new_layout: Layout. This is because this method does not support changing the alignment.

When (re)allocation fails (when the method returns null), the caller typically wants to call handle_alloc_error which takes a Layout. This value needs to be created with code like Layout::from_size_align(new_size, layout.align()).unwrap(). It would be better if the value passed to the error handler was already available. This would be the case if the parameters to realloc were old_size: usize, new_layout: Layout instead.

For the Alloc trait, we could:

  • Keep consistency with GlobalAlloc, or
  • Change to old_size: usize, new_layout: Layout, or
  • If we also decide to lift the same-alignment restriction (which should be discussed in Support reallocating to a different alignment? #5), then the signature becomes old_layout: Layout, new_layout: Layout and this issue is moot.
@SimonSapin SimonSapin changed the title GlobalAlloc::realloc’s parameter are backward GlobalAlloc::realloc’s parameters are backward May 3, 2019
@TimDiekmann
Copy link
Member

If we also decide to lift the same-alignment restriction (which should be discussed separately, feel free to file a dedicated issue), then the signature becomes old_layout: Layout, new_layout: Layout and this issue is moot.

I like this option the most, as this may be valid for arbitrary allocators.

@SimonSapin
Copy link
Contributor Author

Alright, I’ve filed #5 for that discussion.

@TimDiekmann TimDiekmann added A-Alloc Trait Issues regarding the Alloc trait Discussion Issues containing a specific idea, which needs to be discussed in the WG labels May 3, 2019
@SimonSapin
Copy link
Contributor Author

#5 is closed, and AllocRef’s grow and shrink methods still take layout: Layout, new_size: usize. Should they be changed to old_size: usize, new_layout: Layout?

@TimDiekmann
Copy link
Member

I find it confusing when the new_ parameter gets an alignment, but that has to match the alignment of the old layout. Surely it is more convenient when one already has the layout passed to the method, but that Layout also has to be constructed beforehand.

@TimDiekmann
Copy link
Member

TimDiekmann commented Aug 11, 2020

As the AllocRef trait changed a lot since this issue was opend, I opened #69 and closing this in favor of the linked issue.

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 Discussion Issues containing a specific idea, which needs to be discussed in the WG
Projects
None yet
Development

No branches or pull requests

2 participants