rust-lang / rust-lang/libs-team
ACP: Remove `T: Send + Sync` bound from `Arc::<dyn Any + Send + Sync>::downcast<T> where T: Any + Send + Sync`
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 178
- Forks
- 28
- Avg merge
- 15m
- Merged PRs (30d)
- 1
Description
Proposal
Problem statement
Arc::downcast is currently defined like this:
impl Arc<dyn Any + Send + Sync> {
pub fn downcast<T>(self) -> Result<Arc<T, A>, Self>
where
T: Any + Send + Sync, // (ACP author's note) this `Send + Sync` is unnecessary
{ ... }
}
The Send + Sync bound on T is not necessary. Note that in the error branch, the Arc<dyn Any + Send + Sync> is returned unchanged, so the Send + Sync bound never plays any role in the implementation or return type of the method.
This can also be seen on the related is, downcast_ref and downcast_mut functions on dyn Any + Send + Sync, as well as the analogous Box::<dyn Any + Send + Sync>::downcast, which all do not have these bounds.
In my view, the bound should have justification for existing, or be removed.
As a result of its existence, users have to mimic it on their own generics when downcasting an Arc<dyn Any + Send + Sync>, or reimplement the functionality using unsafe code.
Motivating examples or use cases
Here (Link1, Link2, Link3) are some examples of functions I found using very basic code search where the Send + Sync bound from downcast leaks into the generics of a function.
Note that any generic registry based on Arc has this bound leak into its retrieval API, which is what these examples all fall under. In some of these, the bound even leaks into the generics of the method of a "retrieval trait", which prevents it from having another implementation that works for non-Send + Sync types.
The bound also creates confusion, because users may think that there is some soundness reason for why the destination type must be Send + Sync, even though there is none.
Here is a particularly interesting example of a user directly complaining about this in a comment above a manual unsafe implementation, though they would have required an Arc::<dyn Any>::downcast method to avoid the manualy implementation (which is outside the scope of this proposal).
Solution sketch
Delete 14 bytes from library/alloc/src/sync.rs. Probably another 14 on the unstable downcast_unchecked method, which has the same bound.
Alternatives
N/A
Links and related work
Some discussion 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
Read library/alloc/src/sync.rs and the linked discussion first; compare Arc downcast with the related dyn Any and Box APIs named in the issue. Done means the unnecessary bounds, including the unstable downcast_unchecked bound if accepted, are removed and the API change has libs-api approval.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100