On Wed, Aug 07, 2024 at 08:01:00AM +0000, Benno Lossin wrote: > On 07.08.24 09:51, Alice Ryhl wrote: > > On Wed, Aug 7, 2024 at 9:49 AM Benno Lossin <benno.lossin@xxxxxxxxx> wrote: > >> On 07.08.24 01:01, Danilo Krummrich wrote: > >>> On Tue, Aug 06, 2024 at 07:47:17PM +0000, Benno Lossin wrote: > >>>> On 05.08.24 17:19, Danilo Krummrich wrote: > >>>>> +impl<T, A> Box<T, A> > >>>>> +where > >>>>> + T: ?Sized, > >>>>> + A: Allocator, > >>>>> +{ > >>>>> + /// Constructs a `Box<T, A>` from a raw pointer. > >>>>> + /// > >>>>> + /// # Safety > >>>>> + /// > >>>>> + /// `raw` must point to valid memory, previously allocated with `A`, and at least the size of > >>>>> + /// type `T`. > >>>> > >>>> With this requirement and the invariant on `Box`, I am lead to believe > >>>> that you can't use this for ZSTs, since they are not allocated with `A`. > >>>> One solution would be to adjust this requirement. But I would rather use > >>>> a different solution: we move the dangling pointer stuff into the > >>>> allocator and also call it when `T` is a ZST (ie don't special case them > >>>> in `Box` but in the impls of `Allocator`). That way this can stay as-is > >>>> and the part about ZSTs in the invariant can be removed. > >>> > >>> Actually, we already got that. Every zero sized allocation will return a > >>> dangling pointer. However, we can't call `Allocator::free` with (any) dangling > >>> pointer though. > >> > >> The last part is rather problematic in my opinion, since the safety > >> requirements of the functions in `Allocator` don't ensure that you're > >> not allowed to do it. We should make it possible to free dangling > >> pointers that were previously "allocated" by the allocator (ie returned > >> by `realloc`). > >> Maybe we do need an `old_layout` parameter for that (that way we can > >> also `debug_assert_eq!(old_layout.align(), new_layout.align())`). > > > > The std allocators generally prohibit zero sized allocations, so it > > seems sensible for us to do the same? > > I never understood why they do that, the stdlib `Allocator` trait has > all the information it needs to detect zero-sized allocations, so it > could just return dangling pointers. I don't see the point of > duplicating the zero-sized logic in `Box` and `Vec`... I think it's simpler and less error prone. Besides that, I think the stdlib `Box` allows to call `from_raw` with any random pointer for ZST, which the allocator API can't catch. > > --- > Cheers, > Benno >