Remove the various Default traits

[lachlansneff]

As discussed in the embedded-hal meeting, the default traits and their corresponding impls interfere with upstream implementations.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ryankurte (or someone else) soon.

Please see the contribution instructions for more information.

[GrantM11235]

Here is the example that I brought up in the meeting:

struct GenericSpiDevice<W>(core::marker::PhantomData<W>);

impl<W> embedded_hal::blocking::spi::Write<W> for GenericSpiDevice<W> {
    type Error = ();
    fn write(&mut self, _: &[W]) -> Result<(), Self::Error> {
        todo!()
    }
}

error[E0119]: conflicting implementations of trait `embedded_hal::prelude::_embedded_hal_blocking_spi_Write<_>` for type `GenericSpiDevice<_>`:
 --> src/lib.rs:3:1
  |
3 | impl<W> embedded_hal::blocking::spi::Write<W> for GenericSpiDevice<W> {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: conflicting implementation in crate `embedded_hal`:
          - impl<W, S> _embedded_hal_blocking_spi_Write<W> for S
            where S: embedded_hal::blocking::spi::write::Default<W>, W: Clone;
  = note: downstream crates may implement trait `embedded_hal::blocking::spi::write::Default<_>` for type `GenericSpiDevice<_>`

---

I don't think that embedded_hal::blocking::digital::toggleable::Default causes the same problem because it doesn't have a generic type parameter.

[burrbull]

I don't understand how they can interfere.
Does GenericSpiDevice<W> implement Default<W>?

[GrantM11235]

That took me a while to figure out too. Apparently, a downstream crate foo with a type Bar could impl embedded_hal::blocking::spi::write::Default<foo::Bar> for GenericSpiDevice<foo::Bar>.

[burrbull]

That took me a while to figure out too. Apparently, a downstream crate foo with a type Bar could impl embedded_hal::blocking::spi::write::Default<foo::Bar> for GenericSpiDevice<foo::Bar>.

Just delete this impl.

[GrantM11235]

That is a hypothetical impl, it doesn't exist. The mere fact that it could exist (in a different crate) is enough for rustc to forbid impl<W> embedded_hal::blocking::spi::Write<W> for GenericSpiDevice<W>.

[ryankurte]

ahh i've been caught by this before, you can avoid the problem by removing the generic over word length and implementing over concrete u8/u16/whatever:

struct GenericSpiDevice<W>(core::marker::PhantomData<W>);

impl embedded_hal::blocking::spi::Write<u8> for GenericSpiDevice<u8> {
    type Error = ();
    fn write(&mut self, _: &[u8]) -> Result<(), Self::Error> {
        todo!()
    }
}

impl embedded_hal::blocking::spi::Write<u16> for GenericSpiDevice<u16> {
    type Error = ();
    fn write(&mut self, _: &[u16]) -> Result<(), Self::Error> {
        todo!()
    }
}

in practice you usually can't do much with the generic trait as they're not object-safe so you'll basically always need to take this approach anyway, does that solve your issue?

[GrantM11235]

My earlier example is a bit contrived, what I was actually trying to do is create an adapter that takes a blocking::spi::Write and implements blocking::spi::WriteIter.

I could copy-paste or use a macro to impl this separately for u8 and u16, but that would mean that my adapter wouldn't work for 9-bit words for example.

In my opinion there are also other good reasons to remove the default trait markers before v1.0. The way they are implemented is confusing and feels like a hack, and they only save the HALs from having to write a few lines of trivial code.

[eldruin]

FWIW, I'm fine with removing the default impls. As commented here and elsewhere, they add little value, can be slightly problematic as commented above and assume nb as the "main" traits, which we will deprecate at some point.

[Dirbaio]

I'm in favor of removing the Default traits, even if the above mentioned conflicting impl issues can be worked around.

1. It establishes the nb traits as the True Traits and the blocking traits as derived ones. It encourages HAL writers to impl only the nb traits and base everything off that. nb is incompatible with lots of nice stuff, such as DMA impls. We might even want to deprecate nb in the future.
2. It doesn't make for very nice docs. Example. Ideally that should show Spi impls embedded_hal::blocking::spi::Transfer, but it doesn't. The Default implementation detail leaks to the end user.

[therealprof]

Excellent, thanks.

bors r+

[bors]

Build succeeded:

• ci-linux (1.35.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• fmt