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

Only include AtomicWaker with std build #845

Closed
wants to merge 1 commit into from

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Mar 7, 2018

Workaround for #842

This is not my preferred solution, but it's the best solution I can think of that works on stable rust

@cramertj
Copy link
Member

cramertj commented Mar 13, 2018

I'd like to keep AtomicWaker usable on platforms that support it, even if they aren't using no_std. Do you know of specific platforms that don't support AtomicUsize for which you could cfg out AtomicWaker?

@Nemo157
Copy link
Member Author

Nemo157 commented Mar 13, 2018

The only platform I know of/care about right now is thumbv6m-none-eabi. I assume there are other embedded platforms that don’t, but I can start with just excluding that.

@Nemo157
Copy link
Member Author

Nemo157 commented Mar 13, 2018

So, #[cfg(not(target = "thumbv6m-none-eabi"))] doesn't seem to be supported (not listed in the reference and using it doesn't exclude the code from compiling).

#[cfg(not(target_arch = "arm"))] works, but is far too broad since (for example) that includes armv7-apple-ios which supports atomic instructions.

As mentioned in the linked ticket #[cfg(target_has_atomic = "ptr")] is the correct solution, but that requires the unstable cfg_target_has_atomic feature.

None of the other cfg options seem relevant.

@Nemo157
Copy link
Member Author

Nemo157 commented Mar 13, 2018

Since #854 introduced nightly feature flags, I've pushed an alternative branch that uses #[cfg_attr(feature = "nightly", cfg(target_has_atomic = "ptr"))] to disable AtomicWaker when building with the nightly feature for a target that doesn't support it. If you'd prefer that I can replace this PR with it.

@cramertj
Copy link
Member

cramertj commented Mar 13, 2018

@Nemo157 That sounds great to me, so long as you're fine sticking to nightly when using thumbv6m-none-eabi. (it's better than not being able to use it at all, certainly!)

@Nemo157
Copy link
Member Author

Nemo157 commented Mar 13, 2018

I have many other things tying me to nightly at the moment, so 🤷‍♂. I’ll wait for #871 to be merged then open a PR to avoid conflicts.

@Nemo157 Nemo157 closed this Mar 13, 2018
@Nemo157 Nemo157 deleted the atomic-std-only branch February 16, 2019 16:17
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