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

Make DowncastSync Optional #22

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

bushrat011899
Copy link
Contributor

Objective

Currently, downcast-rs does not compile on platforms which lack Arc support, such as thumbv6m-none-eabi (e.g., the Raspberry Pi Pico). Crates like portable-atomic-util can provide a compatibility shim, but without RFC #982, and RFC #3519, the ergonomics would be vastly different with and without portablie-atomic.

Solution

Added a feature flag for DowncastSync, sync. Since it is the only part of downcast-rs that relies on Arc, disabling just it alone is sufficient to allow compilation on thumbv6m-none-eabi and other atomically challenged platforms. This does constitute a breaking change for users who disable default features, as they will have to enable the sync feature to retain existing functionality.

Alternatives

Wait for the above RFCs to be stabilised and then use portable-atomic instead.

Notes

This is my first time contributing to this project. Please let me know if there's anything I can do to assist with evaluating this PR.

The `Arc` usage in `DowncastSync` prevents compilation on platforms like `thumbv6m-none-eabi`. By placing it behind a feature flag, `sync`, we can allow compilation of the supported subset.
@marcianx
Copy link
Owner

Thanks for your contribution! And thanks for being explicit about the backward-compatibility hazard. I suppose I could publish a 2.0 with this. I'll see if there are any other similar backward-incompatible-ish changes from #14 that now might be a good time to incorporate. Do you need this published in crates.io in a rush?

@marcianx marcianx merged commit 9ad1dfb into marcianx:master Dec 19, 2024
2 checks passed
@bushrat011899
Copy link
Contributor Author

Thanks for merging it so quickly! Not a rush at all. We're able to work around the issue in Bevy and we won't be publishing a crate that'd need this change for at least a couple months, so no time pressure at all.

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

Successfully merging this pull request may close these issues.

2 participants