Without `critical-section` feature, Rust gets confused about missing `take()`

[adamgreig]

Currently the Peripherals::take() method is gated behind the critical-section optional dependency (and thus feature). Without the feature enabled, the method isn't available, because it requires the critical-section crate and we wanted to make that dependency optional (#651).

However it turns out if you call take() on a struct that doesn't have that method, Rust thinks you wanted to call the method on the Iterator trait that's imported by default, and then you get a pretty confusing error message about Peripherals not implementing Iterator (example):

struct Foo;

fn main() {
    let f = Foo {};
    f.take();
}

error[[E0599]](https://doc.rust-lang.org/stable/error-index.html#E0599): `Foo` is not an iterator
 --> src/main.rs:5:7
  |
1 | struct Foo;
  | ----------
  | |
  | method `take` not found for this struct
  | doesn't satisfy `Foo: Iterator`
...
5 |     f.take();
  |       ^^^^ `Foo` is not an iterator
  |
  = note: the following trait bounds were not satisfied:
          `Foo: Iterator`
          which is required by `&mut Foo: Iterator`
note: the following trait must be implemented
  = help: items from traits can only be used if the trait is implemented and in scope
  = note: the following trait defines an item `take`, perhaps you need to implement it:
          candidate #1: `Iterator`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `playground` due to previous error

This seems to really throw people off because there's no indication that take() didn't exist or was feature-gated. I wonder if we should either...

1. Make critical-section a hard dependency and always provide take(); it should build fine and only error about a missing c-s implementation if take() is actually called, in which case at least the error message is better, or
2. Always implement a take(), but if the critical-section feature is not enabled, use compile_error!() to emit a custom compiler error instead of the very confusing one we currently get.

[Emilgardis]

Yes I've been stung by this too after the change 😅

I think using compile_error!() could be a good idea, but how would we make that work in a good way so that critical-section doesn't become a needed dependency? compile_error!() causes a compile error whenever it is present anywhere as a result of a macro or normal source generation, i.e simply having fn foo() {compile_error!()} causes a compile time error, even if it's never used in a binary

I think having it be a missing symbol is the better alternative, but perhaps there is another way to solve it.

[martinmortsell]

I ran into this issue recently, and I noticed that our pac (without critical-section) is missing take() for Peripherals but take() is present for CorePeripherals. Should this be considered a bug? In my mind CorePeripherals and Peripherals should behave in the exact same way.

[martinmortsell]

Nevermind, I looked through the code and the impl Peripherals for CorePeripherals comes from the cortex-m crate. So, it might be somewhat inconsistent but it's not really connected to svd2rust specifically.