Repository navigation
Reland debug impl for fieldless enums with concatenated string without using discriminant_value #162535
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
Reland debug impl for fieldless enums with concatenated string without using discriminant_value #162535
Changes from all commits
64639c6
8ca2abb
80a3be3
0076e09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2619,6 +2619,19 @@ impl<'a> Formatter<'a> { | |
| builder.finish() | ||
| } | ||
|
|
||
| /// Shrinks `derive(Debug)` code, for faster compilation and smaller binaries. | ||
| /// For C-like enums with concatenated variant name strings. | ||
| /// Names should be separated by space. | ||
| #[doc(hidden)] | ||
| #[unstable(feature = "fmt_helpers_for_derive", issue = "none")] | ||
| pub fn debug_c_like_enum_write_str<'b>( | ||
| &'b mut self, | ||
| names: &str, | ||
| start_index: usize, | ||
| ) -> Result { | ||
| self.write_str(names[start_index..].split(' ').next().unwrap()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you passed the It would require the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that would erase big part of the win. This version has 2 statements per block, but two index version would have at least 4 I think, I'd have to check. There's a way to do this by packing the two indexes into one number, but it felt like a pretty convoluted solution just for the debug impl. I'll maybe try it later though.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this line causes a significant binary size regression, particularly on embedded, because it pulls in panic and formatting machinery. It is not guaranteed that the In https://godbolt.org/z/z8rTKe6j6 (LLM-generated but the issue is pretty clear), when using "Link to binary", the output size grows from ~1000 lines to ~2600. Putting both sides into a diff viewer, I see that In many applications those might be included anyway, but especially on embedded (or wasm) binary size is a real constraint. This was reported in #165189.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
|
|
||
| /// Creates a `DebugTuple` builder designed to assist with creation of | ||
| /// `fmt::Debug` implementations for tuple structs. | ||
| /// | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is slightly confusing that the example only has three arms, and so couldn't actually be produced. Expand it to have 6 arms, and use
...to omit repetitive lines?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure I understood the sentence right, but I just made the example with 6 arms. I guess more explicit example doesn't hurt