Open orlp opened 3 months ago
I think Box::map
is good but I'm not sure about Arc::map
which requires a Clone bound of the inner type.
An alternative is making Arc::map
take a FnOnce(&T) -> U
rather than FnOnce(T) -> U
.
f
doesn't need to own the inner value
Proposal
Problem statement
Suppose you a value contained in a
Box
,Rc
, orArc
. It is quite natural to want to map this value to a different value, but still wanting the result to stay in aBox
,Rc
orArc
. Currently the easiest and most natural way of doing this always involves an allocation, whether this is necessary or not:Motivating examples or use cases
Whenever you have both your input and output as a
Box
,Rc
orArc
it is likely that you experience this problem. A personal example of mine was from a Domain Specific Language represented usingArc
s. A simplified example is doing a recursive expression replacement of-x
with0 - x
:Here we do an allocation for each node in the expression tree, when in a lot of cases the allocation could have been re-used. If I had used
Box
es instead ofArc
s every single allocation could have been re-used, only adding an allocation for the constantNumber(0)
added to the expression tree wheneverNeg
is substituted.Solution sketch
I propose we add the following functions to
Box
,Rc
andArc
:The definitions for
Rc
are identical, the definitions forBox
would also be the same except without aT: Clone
bound.While the shown example implementations work, they still would cause needless allocations. However in the standard library we can use unsafe code internally, and re-use the allocation (after
{Rc, Arc}::make_mut
has been called) with atransmute
wheneverT
andU
(orT
andR::Output
fortry_map
) have the same size and align.With
Arc::map
the above example simplifies to:Alternatives
Arc::new
and such.unsafe
code yourself (don't mess up the case whenf
panics!).replace_with
crate, if yourT
hasDefault
and don't mind a tiny bit of overhead fromDefault
being temporarily placed inside the container.replace_with
crate, if you don't mind an otherwise recoverable panic be turned into an abort.map_box
crate if you only care aboutBox
and don't need theTry
version.While the latter case (
map_box
) could be extended to cover the other cases, I feel such a basic and core functionality should be in the standard library instead of a third-party crate.Links and related work
https://docs.rs/replace_with/latest/replace_with https://docs.rs/map_box/ https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/.7BBox.2C.20Rc.2C.20Arc.7D.3A.3Amap.20and.20.7BBox.2C.20Rc.2C.20Arc.7D.3A.3Atry_map
What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
Second, if there's a concrete solution: