Open kornelski opened 1 month ago
r? @estebank
rustbot has assigned @estebank. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r?
to explicitly pick a reviewer
Some changes occurred in cfg and check-cfg configuration
cc @Urgau
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
I'd love to see this happen.
I have a suggestion for a refinement: In the none
case, I think it might be better to write a constant string rather than the empty string. While this does slightly negate the code-size benefit, it will aid debuggability to get a recognizable string rather than a mysteriously empty one (because many different bugs can cause empty strings, and the empty string might even be used intentionally with the expectation that no value's Debug
representation is the empty string).
This constant string should be "[object Object]"
— just kidding. Maybe something like "<disabled>"
, "{debug-fmt-detail=none}"
, or perhaps, for minimality while still using Rust idiom,"_"
.
The job mingw-check-tidy
failed! Check out the build log: (web) (plain)
Some changes occurred in tests/ui/check-cfg
cc @Urgau
The job mingw-check-tidy
failed! Check out the build log: (web) (plain)
@kpreid But none
isn't merely writing an empty string. It makes Debug::fmt
an empty function that doesn't touch the Formatter
, which makes it possible to optimize it entirely.
There is LoweringContext::lower_format_args
that maybe could replace {:?}
with a string literal. However, it needs to keep the arg expressions for their side effects. Perhaps the args could be changed to something like drop(&arg)
or (&arg, "").1
, but I wasn't able to figure out how to transform the expressions. It needs to modify FormatArgs
with ast::Expr
, but the lower_format_args
seems to have access only to hir::Expr
.
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
Some changes occurred in src/doc/rustc/src/check-cfg.md
cc @Urgau
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
:umbrella: The latest upstream changes (presumably #124507) made this pull request unmergeable. Please resolve the merge conflicts.
The job mingw-check-tidy
failed! Check out the build log: (web) (plain)
:umbrella: The latest upstream changes (presumably #124345) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #124726) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #125219) made this pull request unmergeable. Please resolve the merge conflicts.
@rfcbot fcp merge
Team member @estebank has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
:umbrella: The latest upstream changes (presumably #124417) made this pull request unmergeable. Please resolve the merge conflicts.
I suggest dropping the "detail" from the name.
We have similar options like -C debuginfo
and they are not called -C debuginfo-level
or -C debuginfo-detail
, it's already clear from the possible values (and -C
already says that it's about codegen).
(Also maybe debug_fmt
-> fmt_debug
to match the trait/macro path std::fmt::Debug
.)
The job mingw-check-tidy
failed! Check out the build log: (web) (plain)
The job x86_64-gnu-llvm-17
failed! Check out the build log: (web) (plain)
I'd like to propose a new option that makes
#[derive(Debug)]
generate no-op implementations that don't print anything, and makes{:?}
in format strings a no-op.There are a couple of motivations for this:
Debug
implementations. It's hard to avoid that without compiler's help, because debug formatting can be used in many places, including dependencies, and their loggers, asserts, panics, etc.panic_immediate_abort
). There are targets like Web WASM or embedded where users pay attention to binary sizes.Debug
format implementation in unwise ways (e.g. trying to get data unavailable via public interface, or using it as a serialization format). Because current Rust's debug implementation doesn't change, there's a risk of it becoming a fragile de-facto API that won't be possible to change in the future. An option that "breaks" it can act as a grease.This implementation is a
-Z debug-fmt-detail=opt
flag that takes:full
— the default, current state.none
— makes derivedDebug
and{:?}
no-ops. Explicitimpl Debug for T
implementations are left unharmed, but{:?}
format won't use them, so they may get dead-code eliminated if they aren't invoked directly.shallow
— makes derivedDebug
print only the type's name, without recursing into fields. Fieldless enums print their variant names.{:?}
works.The
shallow
option is a compromise between minimizing theDebug
code, and compatibility. There are popular proc-macro crates that useDebug::fmt
as a way to convert enum values into their Rust source code.There's a corresponding
cfg
flag:#[cfg(debug_fmt_detail = "none")]
that can be used in user code to react to this setting to minimize customDebug
implementations or remove unnecessary formatting helper functions.