rust-lang / rust-lang/libs-team
Add generic `downcast` functions to combat combinatoric explosion
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 178
- Forks
- 28
- Avg merge
- 15m
- Merged PRs (30d)
- 1
Description
Proposal
This proposal supersedes #861 using the Unsize-free implementation suggested by @danielhenrymantilla (thanks!)
Problem statement / Motivating examples
The standard library has 3 different downcast methods on Box, one for each of dyn Any, dyn Any + Send, dyn Any + Send + Sync, as well as one for Arc<dyn Any + Send + Sync> and Rc<dyn Any>.
With each auto trait, we have to ask the question if there's a particular combination of bounds that would be useful to provide downcasting on. Other combinations will be left without a downcasting method; some would-be-useful examples include Arc<dyn Any> and Box/Rc/Arc<dyn Error + ...>.
In general, this problem is known as combinatoric explosion, because in the most extreme case, we have to write one method for each possible combination of bounds.
Downstream subtraits of Any/Error may also need to implement their own downcasting methods. downcast-rs is a crate dedicated to generating such methods with over 100 million downloads.
Solution sketch
Add the following associated functions on Box, Rc, and Arc:
impl<T: ?Sized, A: Allocator> Box<T, A> {
pub fn downcast_any<U: 'static>(this: Self) -> Result<Box<U, A>, Self>
where
T: Any
{
if TypeId::of::<U>() == <T as Any>::type_id(&this) {
let (ptr, alloc) = Box::into_raw_with_allocator(this);
Ok(unsafe { Box::from_raw_in(ptr.cast(), alloc) })
} else {
Err(this)
}
}
pub fn downcast_error<U: 'static>(this: Self) -> Result<Box<U, A>, Self>
where
T: Error + 'static
{ /* Same thing, but using `<T as Error>::type_id`... */ }
}
impl<D: ?Sized, A: Allocator> Arc<D, A> { ... }
impl<D: ?Sized, A: Allocator> Rc<D, A> { ... }
This implementation works for any combination of bounds on Any, as well as on any combination of bounds on every trait Subtrait: Any (both likewise for Error), without code repetition. Existing downcast methods can also be trivialized by just calling the new functions, instead of the current underlying downcast_unchecked method.
On references
Equivalent functions on &(mut) dyn Any + ... are not needed, as the only purpose for subtrait-specific methods is to return the original smart-pointer in case the downcast does not succeed. For references, this is unnecessary due to reborrowing, as indicated by downcast_ref/downcast_mut only returning an Option.
Guarding against improper uses
We probably should guard against calls to these functions on T that are not dyn Subtraits of Any/Error in some form. The signature as proposed here allows downcasting anything 'static, including types that don't make sense. If you put in a Box<dyn UnrelatedTrait>, it'll just get the type id of dyn UnrelatedTrait itself, and the downcast will fail. We can alleviate this one of the following ways:
- Lint against such uses. Calls to
Any::type_idalready have the same issues, except for them, calling<dyn UnrelatedTrait as Any>::type_idmay be intentional (which is why there's only a lint fortype_id_on_box). Fordowncast_any, it's clear what the intended uses are.- We definitely should lint against
Tthat are/might be!Sized + !Unsize<dyn Any/Error>.- This includes all the
dyn UnrelatedTraits, but also things like[I](which someone might try to downcast to an array. no reason to allow this type here)
- This includes all the
- We may allow
T: Sizedas a valid use-case. The functions essentially attempt to castT = Uin this case. If we do not consider this a valid use-case, we lint against uses withTthat are/might beSized.- This is probably never a valid use-case for
downcast_errorthough, sincedowncast_anywill always work in this case.
- This is probably never a valid use-case for
- If we lint against might-be-improper uses, this also lints against generic wrappers around this function, which may be desirable, as they won't have their own lints
- We definitely should lint against
- Assert
<T as Any>::type_id(&this) != TypeId::of<T>()- This assertion fails for all non-dyn-subtraits of
Any/Error, includingSizedtypes
- This assertion fails for all non-dyn-subtraits of
- Use compiler magic to check that
Tis a dyn subtrait ofAny, perhaps post-mono, at runtime, or through a trait bound.
If this has swayed you into thinking that adding these functions to std is not a good idea, consider that downstream implementations (which can already be written for downcast_any) don't get to guard against improper uses using lints or compiler magic! This is something only std can do.
Alternatives
Not doing this
Instead, we can provide SmartPtr::<dyn Any/Error>::downcast only and leave users to write code like this whenever they need to downcast from a specific Any subtrait:
if (&*x as &dyn Any).is::<T>() {
Ok((x as SmartPtr<dyn Any>).downcast().unwrap()) // returns `SmartPtr<dyn Any>` in the error case
} else {
Err(x) // but we want to keep it as `SmartPtr<dyn Subtrait>`
)
One issue with this (besides looking awful) is that we don't currently have such a method for Arc<dyn Any>. Adding it with the traditional downcast name would be a breaking change, due to method name resolution, as calls to Arc::downcast by absolute path coerce the self parameter to Arc<dyn Any + Send + Sync> (see #862). Likewise, dyn Error is missing downcasting functionality entirely, for all smart pointer types. Adding a downcast method for it to Arc and Rc is breaking for the same reason as for dyn Any. So these methods would have to be introduced under a new name that is inconsistent from all existing downcast methods.
Naming
Another alternative is to replace the downcast methods with this. This is not possible because the proposed associated functions are not methods. They need a this: Self parameter instead of a self one, since they are implemented Box<DownstreamType> (which would shadow calls to the same name through Deref if added as methods).
Not doing this, but different
Only downcast_error cannot be written in stable code today, since the type_id method on Error is unstable (it would be unsound to override its default implementation). The alternative that arises from this is to provide a type_id_of_error function in the standard library (or make Error::type_id at least callable) and let downstream implement the generic downcasting functions themselves.
Links and related work
General discussion about the downcasting API on zulip.
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):
- We think this problem seems worth solving, and the standard library might be the right place to solve it.
- We think that this probably doesn't belong in the standard library.
Second, if there's a concrete solution:
- We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
- We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
No repository files or tests are named. Start by reading the existing Box, Rc, and Arc downcast APIs and the linked Zulip discussion, then resolve the proposed bounds, naming, and improper-use guard. Done means the libs-api team has agreed on the API shape and the resulting implementation path is clear.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, developer-experience
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100