JelteF / derive_more

Some more derive(Trait) options
MIT License
1.74k stars 123 forks source link

Regression in v1.0.0-beta.7 #392

Closed link2xt closed 3 months ago

link2xt commented 3 months ago

Change https://github.com/deltachat/deltachat-core-rust/pull/5850 caused problems compiling https://github.com/n0-computer/iroh/

To reproduce, clone https://github.com/n0-computer/iroh/, checkout commit 29d2e82a577ebc8cb4029c0df0138fe662031d5c, go into iroh-net folder, run cargo update -p derive_more (it is pinned to v1.0.0-beta.6 in Cargo.lock) and run cargo check.

Compilation fails with this error:

$ cargo check
    Checking iroh-net v0.21.0 (/home/user/src/iroh/iroh-net)
error[E0277]: `iroh_quinn::Accept<'_>` doesn't implement `derive_more::Debug`
   --> iroh-net/src/endpoint.rs:897:10
    |
897 | #[derive(Debug)]
    |          ^^^^^ `iroh_quinn::Accept<'_>` cannot be formatted using `{:?}` because it doesn't implement `derive_more::Debug`
    |
    = help: the trait `derive_more::Debug` is not implemented for `iroh_quinn::Accept<'_>`, which is required by `&iroh_quinn::Accept<'_>: derive_more::Debug`
    = note: required for `&iroh_quinn::Accept<'_>` to implement `derive_more::Debug`
    = note: required for the cast from `&&iroh_quinn::Accept<'_>` to `&dyn derive_more::Debug`
    = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `iroh-net` (lib) due to 1 previous error

iroh devs worked around the problem in PR https://github.com/n0-computer/iroh/pull/2578, but this does not look like an actual fix.

JelteF commented 3 months ago

Okay I looked into this a bit closer, and I'm going to consider this new compilation failure intended behaviour. Before #371 derive_more would put bounds on all the types that were formatted as Debug. But that was actually causing a problem here: Deriving Debug would succeed, but there would be a bound there that would always be false. So a Debug implementation was derived that was never applicable. This was also mentioned in: https://github.com/n0-computer/iroh/pull/2578. A minimal example of this issue is this:

use derive_more::Debug;

struct NoDebug<'a> {
    a: &'a f64,
}

#[derive(Debug)]
struct SomeType<'a> {
    no_debug: NoDebug<'a>,
}

fn main() {}

This now throws an error. Which seems like good behaviour to me. Because then the user will either also derive Debug for NoDebug or they can add #[debug(skip)] to the no_debug field of SomeType. I've created #393 to encode this intentional compilation failure in a compile_fail test.

matheus23 commented 3 months ago

Yeah I think I agree with the changes made in #371. It's unfortunate that a derive_more = "1.0.0-beta.6" dependency broke our users, but tbh, it's on us for not pinning the version bound (=1.0.0-beta.6) while it's a beta version. Our "work around" is honestly how it should've been from the start: We had a bunch of #[derive(Debug)] on structs that had fields that just didn't have Debug implemented (without #[debug(skip)] or the like). I'm glad that leads to errors now instead of working silently (until one wants to use the debug impl).