search

rust-lang / wg-allocators

Home of the Allocators working group: Paving a path for a standard set of allocator traits to be used in collections!
http://bit.ly/hello-wg-allocators
207 stars 9 forks source link

GlobalAlloc::realloc’s parameters are backward #3

Closed SimonSapin closed 4 years ago

SimonSapin commented 5 years ago

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:

TimDiekmann commented 5 years ago

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 commented 5 years ago

Alright, I’ve filed https://github.com/rust-lang/wg-allocators/issues/5 for that discussion.

SimonSapin commented 4 years ago

https://github.com/rust-lang/wg-allocators/issues/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 commented 4 years ago

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 commented 4 years ago

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