embedded-hal icon indicating copy to clipboard operation
embedded-hal copied to clipboard

wip: async: switch to async-fn-in-traits

Open Dirbaio opened this issue 3 years ago • 1 comments
trafficstars

Latest Rust nightlies have somewhat usable async-fn-in-trait support already! :tada:

embassy-nrf updated here https://github.com/embassy-rs/embassy/pull/974

Paprecuts encountered:

  • there's this annoying error playground, workaround is to use the concrete type instead of Self::Error. This is a limitation of all async fns, not just in traits, but it hits especially hard within traits, so I dunno if there's plans to improve it.

async fn return type cannot contain a projection or Self that references lifetimes from a parent scope

  • The SpiDevice trait ICEs, issue filed https://github.com/rust-lang/rust/issues/102310
  • default methods don't work, but there's a PR already https://github.com/rust-lang/rust/issues/102308

Due to the last 2 I've left SpiDevice alone for now.

Dirbaio avatar Sep 26 '22 14:09 Dirbaio

r? @eldruin

(rust-highfive has picked a reviewer for you, use r? to override)

rust-highfive avatar Sep 26 '22 14:09 rust-highfive

It seems the 2 issues with SpiDevice has been fixed in nighly now.

korken89 avatar Oct 20 '22 06:10 korken89

converted all traits, embedded-hal-async no longer explodes when compiling now! I encountered another rustc bug https://github.com/rust-lang/rust/issues/103457 but thankfully we don't need the + 'a anymore, so just removing it worked...

I haven't tried updating embassy yet, it's still possible it explodes with actual use.

Dirbaio avatar Oct 24 '22 01:10 Dirbaio

How are we on this, is this ready to merge? I'm about to start writing some example drivers and would love this :D

If not I'll use a git-dep to this branch.

korken89 avatar Oct 31 '22 13:10 korken89

Since AFIT support is very recent in rustc we should check that it works end-to-end first. embedded-hal-async itself builds without ICEs now, but I haven't tried implementing it in a HAL and using it from a driver yet, it's still possible these things ICE. I wanted to update embassy but haven't found time for it yet. If you want to help test it out by using it as a git dep please go ahead! :)

When we know it works well we can merge it and release it as embedded-hal-async 0.2 :)

Dirbaio avatar Oct 31 '22 15:10 Dirbaio

Super, then I know where it stands! I'll git-dep for now and start hacking :)

korken89 avatar Nov 01 '22 06:11 korken89

No longer builds in nightly-2022-10-29 and newer. Filed https://github.com/rust-lang/rust/issues/103850

Dirbaio avatar Nov 01 '22 21:11 Dirbaio

Also, side note: A few places use T::Error instead of Self::Error because Self is not allowed in async fn return if it has lifetimes. Not a blocker since it can always be workarounded by writing the full Self type, but it can be annoying if it has lots of generics. Will likely be fixed in https://github.com/rust-lang/rust/pull/103491

Dirbaio avatar Nov 01 '22 23:11 Dirbaio

Status update!

  • nightly-2022-11-22 contains fixes for all the blockers
  • Ported embassy-nrf here https://github.com/embassy-rs/embassy/pull/974 . Builds fine, runs fine. The only issue is that nightly also introduces a TAIT regression, but that can be fixed by porting the affected GAT+TAIT traits to AFIT.

So, I'm fairly certain this is ready to go now!

Dirbaio avatar Nov 23 '22 16:11 Dirbaio