Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Fn* blanket impls for Arc #89727

Closed
bsodmike opened this issue Oct 10, 2021 · 5 comments
Closed

Add Fn* blanket impls for Arc #89727

bsodmike opened this issue Oct 10, 2021 · 5 comments

Comments

@bsodmike
Copy link

Hi all,

Apologies if I've opened this issue in the wrong place; I want to work on extending Box<dyn Fn()> to Arc.

The Boxed implementation has been discussed here: #48055 and added in this PR #59500. Can I use this existing RFC or should we open a new one?

The feature gate could be called arc_closure_impls. This discrepancy was discussed by @jonhoo in his latest "Crust of the Rust" episode covering closures etc.

@bsodmike bsodmike changed the title Adding Arc<dyn Fn()> Add Fn* blanket impls for Arc Oct 10, 2021
@ibraheemdev
Copy link
Member

You can just open a pull request adding the implementations, it's not possible to feature gate trait implementations, and you likely don't need an RFC for a smaller change like this. This probably also applies to other pointers as well.

@bsodmike
Copy link
Author

Got it, thanks!

@bsodmike
Copy link
Author

Having looked at this much closer, I've learned the following:

  • Due to Fn's trait-hierarchy and trait bounds defined on FnMut (specifically), we are unable to add the Fn impl to Arc<T>.
  • One idea is to consider Arc<Mutex<T>> but this is not possible as Arc sits in alloc and Mutex is in std.

Based on discussions in Discord:

  • Backward compatibility would break uses of alloc::sync::Arc in #![no_std] crates

My last progress: master...bsodmike:arc_fn_impls

Thanks, in no particular order to Discord users helping out:

  • T-Dark
  • Giuschi
  • @jonhoo for the original idea.

@bsodmike bsodmike reopened this Oct 11, 2021
@bsodmike
Copy link
Author

Thanks to Giuschi, we realized there was another approach. Will open a PR shortly.

@bsodmike
Copy link
Author

Feel free to close this, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants