#[derive(Debug)] by default

Perhaps then that it is not a problem that needs to be solved with a new lint in the compiler itself. If folks are today already providing Debug impls in the vast majority of cases, then a lint might not be worth it at all.

There are some lints enforced by rustc that perhaps would be better in clippy if we were to go back and re-litigate them. But just because that's true doesn't mean we should make it worse by adding more lints that probably don't belong in rustc.

3 Likes

I think such situations are rare, and are easily handled by allow. Could you explain what the problem is with #![allow(missing_debug_for_pub_type)]?

I also don't think we should be optimizing the Rust language to make rustc easier to hack on, at the expense of the broader ecosystem. Especially when the cost to rustc is just adding in a single allow line in lib.rs.

It is the difference between this...

#[derive(Debug)]
struct Foo {
    field1: SomeType,
    field2: SomeOtherType,
    field3: SomeThirdType,
}

...and this:

use std::fmt;

struct Foo {
    field1: SomeType,
    field2: SomeOtherType,
    field3: SomeThirdType,
}

impl fmt::Debug for Foo {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        f.debug_struct("Foo")
            .field("field1", &self.field1)
            .field("field2", &self.field2)
            .field("field3", &"<No Debug>")
            .finish()
    }
}

I have had to write such custom Debug impls multiple times, due to some crate not impling Debug. It's rare, but it does happen.

The code is tedious, verbose, and difficult to get right. But even worse than that is the fact that now you have to keep the Debug impl in sync with the type: it's very easy to add a field to the struct and forget to add it to the Debug impl, whereas derive doesn't have that problem.

I'm honestly very surprised that you don't see this as an issue. I heavily rely upon Debug for debugging (which is its intended use). This issue has been a major footgun for me multiple times, and I doubt I'm alone in this.

5 Likes

How rare? 0.1%, 5%, 10%, ...?

To my knowledge it isn't even possible to set #![allow(...)] on a workspace level right now. But even so, I think forcing users to allow(...) is adding a barrier to hacking, disturbing "flow", when it isn't necessary.

I'm using rustc as a stand-in for large applications with long build times and many internal library crates.

I can't say I find that too onerous or difficult to get right to be honest, and it could be improved:

debug_impl!(Foo, [field1, field2], [field3]);

// expands to the thing above as well as:
let Self { field1: _, field2: _, field3: _ } = self;
// ^~ ERROR missing field4...

A pragmatic solution for this case might be to teach #[derive(Debug)] to recognize a #[debug(omit)] attribute on fields that just pushes <omitted> to the generated debugging output.

12 Likes

I wonder, what if the following worked instead of having to write out the implementation as you did:

#[derive(Debug)]
#[debug_derive_no_debug(field3)]
struct Foo {
    field1: SomeType,
    field2: SomeOtherType,
    field3: SomeThirdType,
}

Or something like that?

1 Like

This could be extended to a string literal like this,

#[derive(Debug)]
struct Foo {
    field1: SomeType,
    field2: SomeOtherType,
    #[debug = "field3 omitted"]
    field3: SomeThirdType,
}

Then debug(omit) could be leave it out of the expansion

#[derive(Debug)]
struct Foo {
    field1: SomeType,
    field2: SomeOtherType,
    #[debug(omit)]
    field3: SomeThirdType, // doesn't show up in `Debug`
}
11 Likes

I highly doubt that it would be 10%. I obviously don't have data on that, but you're the one who's arguing that it's common enough that a single one-time allow line is a burden, so I think the burden of proof is on you.

I think rustc is a rather special case, not typical for large applications. We're talking about applications which:

  • Are quite large.

  • Use internal libraries a lot.

  • Intentionally avoid Debug for the sake of compile times.

  • Don't use any external libraries which use Debug.

  • Never need or want to use Debug.

Most applications don't fit into that category.

And the lack of Debug heavily disturbs flow, much more so than putting in a single one-time allow.

In some cases Debug is critical for debugging programs, because you need to see the internal state of the library, so it's even worse than simply disturbing the flow.

Allow me to spell out clearly the trade-off that we are discussing:

Without a lint

  • Programs which intentionally don't want to use Debug don't need to make any changes.

  • It is not clear whether the author intentionally left out Debug or simply forgot.

  • The entire ecosystem both now and in the future has to pay a continual cost, because authors need to remember to impl Debug, and it is easy to forget.

    And when it is forgotten, this causes problems for all downstream consumers, since they can no longer use derive(Debug), and they can no longer see the internal state of that library when debugging, which can be very important when debugging.

  • Every type which contains a non-Debug type has to create a custom Debug impl.

With a lint

  • Programs which intentionally don't want to impl Debug simply have to add a single #![allow(missing_debug_for_pub_type)] line in their lib.rs. This is a one-time cost. They do not have to pay this cost for every type.

  • This makes it clear when an author intentionally doesn't want to impl Debug, which also helps with maintenance: anybody working on that library will be more careful to not impl Debug, because they know that it is an intentional choice. This helps to prevent people from accidentally adding in a Debug impl.

  • The ecosystem is now better, because there is pervasive Debug, which means derive(Debug) works, and it is possible to debug internal state of libraries.

This feels like a situation where you are prioritizing rustc because you personally work on it, while ignoring the experience of the rest of the Rust ecosystem. Yes rustc is important, but it is not more important than the Rust ecosystem as a whole.

Especially when there is such a clear disparity here between the costs vs benefits.

Do you think adding a macro call specifying the fields for each type is less onerous than a single allow line in lib.rs?

Do you think completely losing all internal debugging state when debugging is less onerous than a single allow line in lib.rs?

Do you think that large applications with many internal libraries that don't want Debug are more common than library authors accidentally forgetting to impl Debug? Especially when there are tens of thousands of crates, so even a 0.1% chance still means many forgotten impls, and this will only get worse over time as the number of crates increase.

That fixes the issue with derive(Debug), but it doesn't allow you to see the library's internal state when debugging, which can be very important for finding bugs.

It is simply very useful to have Debug available, especially in places like WebAssembly which don't have good debuggers, so you rely heavily on println and Debug.

The debugging experience in Rust is simply worse than other languages which have pervasive debugging. I would have thought this would be considered a problem to be fixed, but apparently not.

8 Likes

To toss in another possibility: What about changing cargo new --lib to add #![warn(missing_debug_implementations)]?

Putting the best practice in the template doesn't seem crazy, and it's easy to delete once...

I have the opposite experience. Getting List in C# when I accidentally ToString a List<T> is substantially worse than a compiler error, since it means I need to repro again.

9 Likes

See also the derivative crate: https://mcarton.github.io/rust-derivative/Debug.html

4 Likes

I mainly use .toString() to

  • describe unit test failures
  • compose log messages
  • view variables in debugger

E.g. for text intended for developers' eyes only. Very rarely for anything else. So I find Java's approach adequate. Arguably Debug trait falls into the same category.

1 Like

I just want to add one more thing here, which I forgot to add to the trade-offs:

Since the lint is a warning, not an error, there isn't any "flow disturbance", since your program will still compile just fine. It will even work just fine, without any behavioral changes. That's the point of a lint.

But if a library I'm using lacks Debug, that causes a hard unavoidable compile error, therefore it always causes a heavy disturbance to flow. Especially because the fix for the error is much larger and more involved than just adding a single line of code.

In some cases the "fix" even involves sending a pull request to that library, causing a delay of multiple days! I have personally experienced this multiple times.

I think it's quite clear that is a much worse situation compared to having a warning which is easily dismissed with a single line of code.

3 Likes

I already find the number of warnings we report on incomplete code rather disturbing of my flow. Warnings are less disruptive than errors, but still somewhat disruptive. And this error would trigger all the time unless I make an explicit new annotation on every type (or at least public type) in the code that I'm working on, every time I declare such a type.

Neither solution is optimal. There's a trade off here, its not "quite clear" that one side is "much worse."

9 Likes

Want to add my 5 cents to this discussion.

I'm using Rust mostly for learning and developing small hobby projects at the moment so I can be wrong in my assumptions how compiler works.

Usually it goes for me like this: having some idea I'm writing bunch of structures and code and at some point want to check how it works. So I try to print them and at that moment compiler starts to tell me that I have to implement Debug for a bunch of structures. What is more annoying is that it can be either a temporary code that I'll throw away later or some internal structures that usually don't have to be printed. So compiler waste a time compiling it.

After reading this topic I've got following idea:

  • Compiler should automatically generate Debug implementations for structures only when it is necessary: if user directly or indirectly (by calling function that requires Debug impl) needs it. So this should be some kind of recursive lazy Debug trait implementation.
  • By default compiler doesn't generate Debug impls for crate and its dependencies at all (including std and core crates). Manually implemented Debug should stay as it is.
  • Compiler should ignore existing #[derive(Debug)] attributes and decide itself when to implement it.
  • User can add #[debug(skip)] attribute to the structs/fields that should not be displayed. This can be used to hide some sensitive information (i.e. passwords, cryptographic data, etc.).

I see several pros in this approach:

  • Developers will do less annoying coding.
  • Compiler will do less work so compilation time can decrease.
  • Compiler will generate smaller binaries (at least in Debug mode - in Release compiler removes unused code).

Cons:

  • It can be not so easy to implement such feature in the compiler.
  • It can even increase compilation time.
  • Developers should not forget to add #[debug(skip)] for their sensitive data.

As I said I'm not very experienced in Rust compiler so all of this can be unrealistic to implement.

7 Likes

That is incorrect. As I have said many times, you only need to put a single #![allow(missing_debug_for_pub_type)] line in your lib.rs, and it will silence the warning for your entire crate, both for the current types and future types. So you never need to worry about it ever again. It is a one-time change.

I'm astonished that you are claiming that a hard compile error which can only be fixed by sending a pull request to a library is equivalent to adding a single line in lib.rs once.

I already very clearly listed the trade-offs in my earlier post.

2 Likes

I've suggested enabling the lint in clippy by default even while it's disabled in rustc by default. I feel this would make it more discoverable and would lead to it being used more in practice by exactly the people who want this type of warning, without negative effects on those who don't.

8 Likes

#![forbid(allow)] :grin:

10 Likes

In winapi there are many primitive types such as some function pointers and very long arrays which don't implement Debug, meaning I cannot universally derive Debug on every struct in winapi. However, if I do derive Debug for every struct that is capable of doing so, it adds a significant amount of compile time to winapi due to the sheer number of structs.

If there is some sort of solution for having Debug by default it needs to do so without significantly impacting compile times.

1 Like

So, something like the lazy Debug instantiations that @HaronK was discussing above?

1 Like

What would be a nice solution for that is being able to put the derivation of in this case the Debug trait behind a compile flag, similar to how features work (ie declarable on both the CLI, and as part of the dependency declared in the Cargo.toml of a dependant project) If you don't need the Debug functionality, you can save yourself some compile time. If you do, you can just activate it.

As for the interaction between Debug and raw ptrs: what alternative is there to printing the address of the pointer, or the thing it is pointing to, or a combo of both?

One problem with that approach is that I believe Debug is required for .unwrap(). Some libraries may be using .unwrap(), and all of those calls would need to be replaced with .expect(ā€¦). Perhaps the flag could generate dummy Debug implementations, but Iā€™m not sure how beneficial that would be to compilation times.