embassy-rs / embassy

Modern embedded framework, using Rust and async.
https://embassy.dev
Apache License 2.0
5.15k stars 713 forks source link

#![feature(impl_trait_in_bindings)] has been removed from rustc #318

Closed cnnblike closed 3 years ago

cnnblike commented 3 years ago

Hi, Per https://github.com/rust-lang/rust/commit/e8c04b4386d015e126591c799b562d6a578acc40 and the code on https://github.com/rust-lang/rust/blob/a72c360a30f9a8160e4f40340cecc9b1ce979cd7/compiler/rustc_feature/src/removed.rs#L151-L153 the impl_trait_in_bindings has been removed, which seems like the embassy requires a major change on code. KL

bobmcwhirter commented 3 years ago

I took a quick swing through, and -nrf and -stm32 and their associated example both still compile even without that feature-flag, on our current toolchain nightly.

Can you point me towards where we are currently using that feature?

cnnblike commented 3 years ago

I took a quick swing through, and -nrf and -stm32 and their associated example both still compile even without that feature-flag, on our current toolchain nightly.

Can you point me towards where we are currently using that feature?

Hi, I'm trying to run the example. it shows something like this

   Compiling embassy-traits v0.1.0 (https://github.com/embassy-rs/embassy?branch=master#d83cd3ff)
error[E0557]: feature has been removed
 --> /Users/cnnblike/.cargo/git/checkouts/embassy-9312dcb0ed774b29/d83cd3f/embassy-traits/src/lib.rs:7:12
  |
7 | #![feature(impl_trait_in_bindings)]
  |            ^^^^^^^^^^^^^^^^^^^^^^ feature has been removed
  |
  = note: the implementation was not maintainable, the feature may get reintroduced once the current refactorings are done
bobmcwhirter commented 3 years ago

Yah, we definitely have the feature-flag enabled, but I'm not finding where we actually use that feature upon first glance.

If you're feeling adventurous, comment-out (or remove) all instances of that line (and only that one feature flag) until the compile stops complaining about it. I think we're flipping the flag, but not taking advantage of it. Maybe.

cnnblike commented 3 years ago

I definitely can help with this but I’m not sure whether I’m the right one to do this, like, i have never ran even a helloworld in embassy yet, any expected behavior?

bobmcwhirter commented 3 years ago

We have a few examples in examples/ if any of them match the hardware you might have on hand. cargo run --bin blinky is always a pretty good starting point.

But even earlier, if removing all mentioned of impl_trait_in_bindings and stuff still compiles, that's quite a start.

If it doesn't compile, then paste in the errors you get, and we can track them down.

I do think @Dirbaio tries later nightlies on occasion. We'll get this sorted if it's a larger problem than I'm imagining at the moment.

cnnblike commented 3 years ago

If it doesn't compile, then paste in the errors you get, and we can track them down.

I do think @Dirbaio tries later nightlies on occasion. We'll get this sorted if it's a larger problem than I'm imagining at the moment.

ok, I'll take a look into that, I only have two nrf52833-dk on my hand so I'm not able to test as-is. some modification may be required.

Dirbaio commented 3 years ago

@cnnblike This is already known, fix in progress. To avoid breakages like these, the embassy repo pins the nightly version to a fixed one. You should pin your project to that version too, that's the one that's tested in CI and known working.

As for this particular breakage: the task macro shouldn't require feature(impl_trait_in_bindings), but for some reason the currently pinned nightly does require it. Later nightlies don't, but some ICEs have appeared. See #318

cnnblike commented 3 years ago

oh, looks legit to me. Closing this as there is already a ongoing PR for this.