RawVec
sets capacity incorrectly, causing potentially many bugs
#116580
Labels
A-allocators
Area: Custom and system allocators
A-collections
Area: `std::collections`
A-slice
Area: `[T]`
T-libs
Relevant to the library team, which will review and decide on the PR/issue.
There are two "capacity"s for any successful allocation: the requested capacity and the returned capacity. The
Allocator
trait explicitly allows these to not be the same. However, it appears thatRawVec
assumes they are the same.Everywhere
RawVec
request an allocation, it then sets its internalcap
field for the capacity to be equal to the requested capacity, not the returned capacity The canonical example of this is here, when a newRawVec
is constructed with a given capacity (though the same behaviour also applies when growing allocation). The comment even stateswhich directly contradicts the docs for the
Allocator
trait.In principle, we could design
RawVec
to only store and report the requested capacity and that could be an entirely sound design. However, I worry that the assumption embodied by this comment has wormed its way throughout the standard library and probably the ecosystem. For example, if breaking apart aVec
-based structure to send for FFI, the docs until recently strongly suggested thatVec::capacity
would return the allocated capacity (which is only true if the allocator always returns the requested capacity). In #99790 the docs are currently being updated to reflect that this isn't the case.Another example of where this is problematic is
Vec::to_raw_parts
which clearly assumes thatRawVec::cap
is the allocator returned capacity. I worry that this assumption is implicitly present through a lot of code.I suggest therefore
RawVec
s allocation methods to store the capacity returned from the allocator, unfortunately requiring an extra division to do soRawVec
and structures that use it to ensure that it is clear thatcapacity()
may return more than what was requested, and associated methodscapacity()
returns exactly the allocator returned capacity, and for which datastructures.The alternative that I see is to instead commit to
RawVec::cap
being the requested capacity. In which case, it would be good to review where this may have been assumed elsewhere and what might need to be fixed. Happy to hear views on whether people think this would be a better path.The text was updated successfully, but these errors were encountered: