Futures-rs: Feedback Request: "disable-cas-atomics" feature

[jamesmunns]

Hey all!

I'm looking to use the futures crate on an embedded target with no CAS atomics (specifically thumbv6m-none-eabi), and currently I am required to use the following features which require nightly:

[dependencies.futures]
version = "0.3.14"
default-features = false
features = [ "cfg-target-has-atomic", "unstable" ]

Without these flags, I get the following errors:

error[E0599]: no method named `compare_exchange` found for struct `AtomicUsize` in the current scope
   --> /home/james/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-core-0.3.14/src/task/__internal/atomic_waker.rs:264:14
    |
264 |             .compare_exchange(WAITING, REGISTERING, Acquire, Acquire)
    |              ^^^^^^^^^^^^^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `compare_exchange` found for struct `AtomicUsize` in the current scope
   --> /home/james/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-core-0.3.14/src/task/__internal/atomic_waker.rs:282:42
    |
282 |                     let res = self.state.compare_exchange(
    |                                          ^^^^^^^^^^^^^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `swap` found for struct `AtomicUsize` in the current scope
   --> /home/james/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-core-0.3.14/src/task/__internal/atomic_waker.rs:308:40
    |
308 | ...                   self.state.swap(WAITING, AcqRel);
    |                                  ^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `fetch_or` found for struct `AtomicUsize` in the current scope
   --> /home/james/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-core-0.3.14/src/task/__internal/atomic_waker.rs:375:26
    |
375 |         match self.state.fetch_or(WAKING, AcqRel) {
    |                          ^^^^^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `fetch_and` found for struct `AtomicUsize` in the current scope
   --> /home/james/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-core-0.3.14/src/task/__internal/atomic_waker.rs:381:28
    |
381 |                 self.state.fetch_and(!WAKING, Release);
    |                            ^^^^^^^^^ method not found in `AtomicUsize`

error: aborting due to 5 previous errors

This is due to the target atomic configuration flag currently being nightly-only. Unfortunately, this means that the library that I am working on would also be restricted to nightly-only, or I'd have to fork the futures crate, which would be a compatibility problem.

Would you be open to adding a disable-cas-atomics feature that ALWAYS disables these features (without checking the target cfg)? I believe this should be a straightfoward, but somewhat verbose PR, so I wanted check before working on it.

CC @dirbaio, who has also run into this for embedded async support.

Thanks!

[taiki-e]

I'm okay with adding this as an unstable (but it doesn't require nightly compilers) cfg, but adding this as a feature is unacceptable as the feature must be additive.

see also https://github.com/rust-lang/futures-rs/pull/2294

[jamesmunns]

@taiki-e I'm open to that, how would you suggest structuring the feature? As the current default is assuming that atomics are present, would the correct behavior be to add a use-cas-atomics feature that is default enabled, and let default-features = false notch it out?

I think this would also be a breaking change, though I guess that's allowed under unstable?

[Nemo157]

Moving existing un-flagged API under a new feature flag is a breaking change to existing users of default-features = false. The approach taken in #2294 seems like the only plausible way to approach this as a non-breaking change (or wait for cfg(accessible), but that seems a long way off).

[jamesmunns]

Sorry, @Nemo157, could you clarify what approach you mean? Using a build.rs (potentially with autocfg?) that allows for detecting the presence of CAS atomics instead? I'm feeling a little lost on what the "right" path is to navigate to:

• Have this included
• Not be a breaking change
• Support targets without CAS atomics

[Nemo157]

Yes, autodetecting whether CAS atomics work on the current target or not (literally just reopening #2294 and updating it for any breakage). A feature flag can't be non-breaking because:

• a negative flag would not compose
• a positive flag would be removing API that exists under default-features = false and requiring a new feature flag to enable it

A non-autodetected cfg could be plausible, but isn't a great user-experience, it'd have to be explicitly set in every top-level build that needs it:

RUSTFLAGS='--cfg disable-cas-atomics' cargo build

---

@taiki-e I don't understand what you meant in the last comment on #2294

That said, I agree that there isn't much benefit from this patch, as the user still need to enable unstable features on the user side to actually use cfg_target_has_atomic. So I'll close this for now.

Most users wouldn't need to use cfg_target_has_atomic. Either they assume they're on a non-embedded target and just use AtomicWaker etc. without any cfg (resulting in a compile error if it is used on a target without CAS); or they assume they might be on an embedded target and either don't use AtomicWaker at all, or have their own autocfg/cfg(accessible)/cfg_target_has_atomic gating of their APIs that use it.

[Dirbaio]

either don't use AtomicWaker at all

The problem is even if you don't use it, it fails the build. Simply depending on the futures crate causes the AtomicWaker to compile.

---

An alternative to using autocfg would be manually testing for architectures known not to have CAS in build.rs. defmt does this here. Works fine and is super lightweight.

[Nemo157]

The problem is even if you don't use it, it fails the build. Simply depending on the futures crate causes the AtomicWaker to compile.

Not with the changes in #2294.

[taiki-e]

@Nemo157

I don't understand what you meant in the last comment on #2294

That said, I agree that there isn't much benefit from this patch, as the user still need to enable unstable features on the user side to actually use cfg_target_has_atomic. So I'll close this for now.

Sorry, I don't remember what I was thinking. That (my) comment doesn't seem to be right and I think I was misunderstanding something...

---

@Dirbaio

An alternative to using autocfg would be manually testing for architectures known not to have CAS in build.rs. defmt does this here. Works fine and is super lightweight.

Seems that is a reasonable way. It's more robust and can probably solve most of the problems I mentioned in #2294.
It's not good to need to maintain the list manually, but we can avoid that problem.

[taiki-e]

Filed #2400

[taiki-e]

Released in 0.3.15.