Repository navigation
'unsafe impl<A> Allocator for &A where A: Allocator' is problematic #98232
Description
We have a general Allocator instance for &A where A: Allocator. I think that is a problem. In #98178 I fixed a problem where BTreeMap<T, Global> would actually use &Global as allocator, which leads to performance regression. Or at least, I thought I had fixed that problem -- I just realized that there are some places where it still adds an indirection and therefore adds an unnecessary reference that will be copied around:
rust/library/alloc/src/collections/btree/map.rs
Line 1647 in 3a1e114
I found this by removing the unsafe impl<A> Allocator for &A where A: Allocator + ?Sized instance. I don't know of another way to avoid accidentally introducing such performance regressions.
So maybe that instance should be removed?
Cc @rust-lang/wg-allocators
Activity
[-]'unsafe impl<A> Allocator for &A where A: Allocator + ?Sized' is problematic[/-][+]'unsafe impl<A> Allocator for &A where A: Allocator' is problematic[/+]CAD97 commentedon Jun 18, 2022
See also #94069, #94114 (comment); there're other reasons that this blanket implementation is problematic.
I'd personally be fine removing the impl for the short-to-medium term at least. Long-term/stable needs a bit more consideration at least, though.
Rollup merge of rust-lang#98233 - RalfJung:ref-alloc, r=thomcc
Rollup merge of rust-lang#98233 - RalfJung:ref-alloc, r=thomcc
Amanieu commentedon Jun 24, 2022
@CAD97 I don't think it's problematic if we fix the wording on the
Allocatortrait as I suggested in #94114 (comment).CAD97 commentedon Jun 24, 2022
The impl can be made sound via doc clarifications (which should certainly happen even without the impl provided), yes; I was just noting the issue as a related incidental complexity of the impl.
Rollup merge of #98233 - RalfJung:ref-alloc, r=thomcc
clarfonthey commentedon Oct 4, 2026
I think we're going to have to close this since we decided to stabilize this.