Skip to content

Always implement Debug #4624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Aug 9, 2025

Currently Debug implementations are gated behind the extra-traits feature. My understanding is that historically, this was for two reasons:

  1. Debug implementations for unions were unsound
  2. Concerns about compile time

The first was resolved a while ago by adding a "manual derive" opaque implementation, in 6faa521 ("fix: make Debug impl for unions opaque"). Regarding the second reason, compile times for the current main on my machine are (cleaning in between, with the 2025-08-06 nightly):

  $ cargo build -p libc
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.79s
  $ cargo build -p libc --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.64s
  $ cargo build -p libc --features extra_traits
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.08s
  $ cargo build -p libc --features extra_traits --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.85s

And with this patch applied:

  $ cargo build -p libc
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.89s
  $ cargo build -p libc --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.70s
  $ cargo build -p libc --features extra_traits
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.15s
  $ cargo build -p libc --features extra_traits --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.86s

That is a microbenchmark but it shows that there probably isn't anything to worry about.

Thus, remove the Debug implementation from the feature = "extra_traitts" gating.

Another advantage of doing this is that it should allow many crates to remove the enable for extra_traits, giving us a better idea of who wants Debug vs. who is actually using the Eq/Hash implementations.

Link: #3880

@rustbot
Copy link
Collaborator

rustbot commented Aug 9, 2025

Some changes occurred in solarish module

cc @jclulow, @pfmooney

Some changes occurred in OpenBSD module

cc @semarie

@tgross35
Copy link
Contributor Author

sigh ctest bugs...

Currently `Debug` implementations are gated behind the `extra-traits`
feature. My understanding is that historically, this was for two
reasons:

1. `Debug` implementations for unions were unsound
2. Concerns about compile time

The first was resolved a while ago by adding a "manual derive" opaque
implementation, in 6faa521 ("fix: make Debug impl for unions
opaque"). Regarding the second reason, compile times for the current
`main` on my machine are (cleaning in between, with the 2025-08-06
nightly):

    $ cargo build -p libc
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.79s
    $ cargo build -p libc --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.64s
    $ cargo build -p libc --features extra_traits
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.08s
    $ cargo build -p libc --features extra_traits --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.85s

And with this patch applied:

    $ cargo build -p libc
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.89s
    $ cargo build -p libc --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.70s
    $ cargo build -p libc --features extra_traits
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.15s
    $ cargo build -p libc --features extra_traits --release
     Compiling libc v1.0.0-alpha.1 (~/rust-libc)
      Finished `release` profile [optimized] target(s) in 0.86s

That is a microbenchmark but it shows that there probably isn't anything
to worry about.

Thus, remove the `Debug` implementation from the `feature =
"extra_traitts"` gating.

Another advantage of doing this is that it should allow many crates to
remove the enable for `extra_traits`, giving us a better idea of who
wants `Debug` vs. who is actually using the `Eq`/`Hash` implementations.

Link: rust-lang#3880
@tgross35 tgross35 added the needs-ctest-next Blocked until ctest-next is the default label Aug 10, 2025
@tgross35
Copy link
Contributor Author

@rustbot blocked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants