-
Notifications
You must be signed in to change notification settings - Fork 14k
Add debuginfo_transparent attribute for structs #144223
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
base: main
Are you sure you want to change the base?
Conversation
|
rustbot has assigned @petrochenkov. Use |
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs Some changes occurred in compiler/rustc_attr_data_structures Some changes occurred in compiler/rustc_attr_parsing |
| }; | ||
|
|
||
| if find_attr!(cx.tcx.get_all_attrs(adt_def.did()), AttributeKind::DebuginfoTransparent(..)) { | ||
| let ty = struct_type_and_layout.non_1zst_field(cx).unwrap().1.ty; |
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.
Is there a way to ensure that #[debuginfo_transparent] can only be used when #[repr(transparent)] is also used? And how should I handle the case where all fields are 1zst's?
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've implemented a restriction to #[repr(transparent)] types.
|
|
||
| // gdb-command:print plain_string | ||
| // gdb-check:$1 = alloc::string::String {vec: alloc::vec::Vec<u8, alloc::alloc::Global> {buf: alloc::raw_vec::RawVec<u8, alloc::alloc::Global> {inner: alloc::raw_vec::RawVecInner<alloc::alloc::Global> {ptr: core::ptr::unique::Unique<u8> {pointer: core::ptr::non_null::NonNull<u8> {pointer: 0x[...]}, _marker: core::marker::PhantomData<u8>}, cap: core::num::niche_types::UsizeNoHighBit (5), alloc: alloc::alloc::Global}, _marker: core::marker::PhantomData<u8>}, len: 5}} | ||
| // gdb-check:$1 = alloc::string::String {vec: alloc::vec::Vec<u8, alloc::alloc::Global> {buf: alloc::raw_vec::RawVec<u8, alloc::alloc::Global> {inner: alloc::raw_vec::RawVecInner<alloc::alloc::Global> {ptr: 0x[...], cap: 5, alloc: alloc::alloc::Global}, _marker: core::marker::PhantomData<u8>}, len: 5}} |
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.
This is the improvement this PR causes. Further improvements could be made using future #[debuginfo_flatten] and #[debuginfo_hidden] attributes on RawVec/RawVecInner and the _marker field, which would result in
alloc::string::String {vec: alloc::vec::Vec<u8, alloc::alloc::Global> {ptr: 0x[...], cap: 5, alloc: alloc::alloc::Global, len: 5}}
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.
Similar changes would need to be made for LLDB, but LLDB is broken on the dev-desktop.
This comment has been minimized.
This comment has been minimized.
|
There are changes to the cc @jieyouxu |
This comment has been minimized.
This comment has been minimized.
|
I don't have a strong opinion on whether this attribute should be added or not. I'll delegate this to the lang team, because it approves new attributes anyway. Also, for the implementation maybe find some other reviewer more interested in debuginfo? |
|
For the |
9c64c42 to
04c4fe2
Compare
|
Some changes occurred in compiler/rustc_hir/src/attrs |
This comment has been minimized.
This comment has been minimized.
2fe205c to
43317da
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #145020) made this pull request unmergeable. Please resolve the merge conflicts. |
43317da to
c8752f5
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #145085) made this pull request unmergeable. Please resolve the merge conflicts. |
This attribute causes the struct to be unwrapped at the debuginfo level the same way that repr(transparent) unwraps it at the ABI level. This is useful for preventing types like NonNull and Unique from making the debuginfo harder to read when pretty printers aren't used.
c8752f5 to
45c1dcd
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Just a heads up, LLDB would also need a backwards compatibility fix ( Weighing in from the debug info perspective, this sort of problem is the main purpose of visualizer scripts and it seems a bit odd to cater to not using them. C++ has similar issues, they just aren't as visible because (at least for LLDB) the equivalent pretty printers and SyntheticProviders exist in the debugger itself When using a visualizer script, you can always opt-out of individual visualizers or temporarily "look through" it to see the raw layout (for LLDB, there are display options in the CLI that disable synthetics and the python API has I'm not sure how much of a priority it is for the Rust team that raw debug info to be 1:1 with the Rust representation, but I do think it's somewhat important. Other cases where nodes are generated that don't match the Rust representation (e.g. enums), the justification is hard constraints imposed by the debug info format or the debuggers rather than convenience. With the way this is implemented, AFAICT it is impossible to tell the difference at debug-time between a |
This attribute causes the struct to be unwrapped at the debuginfo level the same way that repr(transparent) unwraps it at the ABI level. This is useful for preventing types like NonNull and Unique from making the debuginfo harder to read when pretty printers aren't used.