Emit line info for generator variants #73460
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
This could be tested by looking at generated LLVM IR metadata. There are a few debuginfo tests in src/test/codegen already, but clang/test/CodeGen might be more useful as a source of inspiration. |
|
r? @ghost while I make some changes |
tmandry
added
S-waiting-on-author
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
tmandry
force-pushed
the
variant-lineinfo
branch
from
84beb18
to
59a0d5e
Compare
tmandry
force-pushed
the
variant-lineinfo
branch
2 times, most recently
from
735b3cc
to
dd883ed
Compare
oli-obk
reviewed
Jun 19, 2020
| @@ -1,5 +1,5 @@ | |||
| // MIR for `main::{{closure}}#0` 0 generator_drop | |||
| // generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } | |||
| // generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], variant_source_info: [SourceInfo { span: $DIR/generator-drop-cleanup.rs:10:15: 10:15 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:13:6: 13:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:13:6: 13:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:12:9: 12:14 (#0), scope: scope[1] }], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } | |||
There was a problem hiding this comment.
Preexisting, but made worse by this PR: we should do a human readable printing of the layout
There was a problem hiding this comment.
I.. might have gone a bit overboard on this one. Let me know what you think :)
|
The impl changes make sense to me, though I can't judge if this is enough to help debuggers make use of it. Please open a tracking issue for this topic so we have something tracking the status and future plans. |
Done: #73524 |
tmandry
added
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 5fbb6ae has been approved by |
bors
added
S-waiting-on-bors
RalfJung
added a commit
to RalfJung/rust
that referenced
this pull request
Jun 20, 2020
Emit line info for generator variants Debuggers should be able to read a generator / async fn state machine and show the line it's suspended at. Eventually, this could grow into an "async stack trace" feature of sorts. While no debugger support this for Rust today, this PR adds the debuginfo necessary for that support to exist. [This gist](https://gist.github.com/tmandry/6d7004fa008684f76809208847459f9b) shows the resulting debuginfo for a simple example. Here's a snippet: ``` 0x00000986: DW_TAG_variant DW_AT_discr_value (0x03) 0x00000988: DW_TAG_member DW_AT_name ("3") DW_AT_type (0x000009bc "Suspend0") DW_AT_decl_file ("/home/tmandry/code/playground/generator-simple.rs") DW_AT_decl_line (6) DW_AT_alignment (8) DW_AT_data_member_location (0x00) ``` The file and line have been added here. The line currently points to the beginning of the statement containing the yield (or await), because that's what the MIR source info points to for the yield terminator. (We may want to point to the yield or await line specifically, but that can be done independently of this change.) Debuggers don't know how to use this kind of info yet. However, we're hoping to experiment with adding such support to Fuchsia's debugger. It would be exciting if someone were interested in adding similar to support to gdb/lldb. r? @oli-obk cc @eddyb @jonas-schievink Part of rust-lang#73524.
bors
removed
the
S-waiting-on-bors
bors
added
the
S-waiting-on-author
tmandry
force-pushed
the
variant-lineinfo
branch
3 times, most recently
from
49a3457
to
0c523e3
Compare
Don't be so reliant on particular line ordering (though FileCheck makes this hard in general, IMO). Also disable for MSVC.
tmandry
force-pushed
the
variant-lineinfo
branch
from
0c523e3
to
33c6cb7
Compare
|
Rebased and got tests working. I've found using FileCheck on LLVM IR to be quite brittle for debuginfo. It's really impossible to write good tests that are resilient to trivial reorderings. We should consider support for checking the output of llvm-dwarfdump, which is a lot more stable. In the meantime I've split out the generator and async fn into separate tests, which helps, but still doesn't let me test everything I want. |
..which doesn't use variant types.
This keeps FileCheck from tripping over unimportant differences in codegen.
tmandry
force-pushed
the
variant-lineinfo
branch
from
33c6cb7
to
887fbd9
Compare
|
@bors r=oli-obk |
|
📌 Commit 887fbd9 has been approved by |
bors
added
S-waiting-on-bors
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Jun 26, 2020
Emit line info for generator variants Debuggers should be able to read a generator / async fn state machine and show the line it's suspended at. Eventually, this could grow into an "async stack trace" feature of sorts. While no debugger support this for Rust today, this PR adds the debuginfo necessary for that support to exist. [This gist](https://gist.github.com/tmandry/6d7004fa008684f76809208847459f9b) shows the resulting debuginfo for a simple example. Here's a snippet: ``` 0x00000986: DW_TAG_variant DW_AT_discr_value (0x03) 0x00000988: DW_TAG_member DW_AT_name ("3") DW_AT_type (0x000009bc "Suspend0") DW_AT_decl_file ("/home/tmandry/code/playground/generator-simple.rs") DW_AT_decl_line (6) DW_AT_alignment (8) DW_AT_data_member_location (0x00) ``` The file and line have been added here. The line currently points to the beginning of the statement containing the yield (or await), because that's what the MIR source info points to for the yield terminator. (We may want to point to the yield or await line specifically, but that can be done independently of this change.) Debuggers don't know how to use this kind of info yet. However, we're hoping to experiment with adding such support to Fuchsia's debugger. It would be exciting if someone were interested in adding similar to support to gdb/lldb. r? @oli-obk cc @eddyb @jonas-schievink Part of rust-lang#73524.
This was referenced Jun 26, 2020
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 26, 2020
…arth Rollup of 14 pull requests Successful merges: - rust-lang#72617 (Add a fast path for `std::thread::panicking`.) - rust-lang#72738 (Self contained linking option) - rust-lang#72770 (Implement mixed script confusable lint.) - rust-lang#73418 (Add unstable `core::mem::variant_count` intrinsic) - rust-lang#73460 (Emit line info for generator variants) - rust-lang#73534 (Provide suggestions for some moved value errors) - rust-lang#73538 (make commented examples use valid syntax, and be more consistent ) - rust-lang#73581 (Create 0766 error code) - rust-lang#73619 (Document the mod keyword) - rust-lang#73621 (Document the mut keyword) - rust-lang#73648 (Document the return keyword) - rust-lang#73673 (Fix ptr doc warnings.) - rust-lang#73674 (Tweak binop errors) - rust-lang#73687 (Clean up E0701 explanation) Failed merges: - rust-lang#73708 (Explain move errors that occur due to method calls involving `self` (take two)) r? @ghost
|
☔ The latest upstream changes (presumably #73746) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Debuggers should be able to read a generator / async fn state machine and show the line it's suspended at. Eventually, this could grow into an "async stack trace" feature of sorts. While no debugger support this for Rust today, this PR adds the debuginfo necessary for that support to exist.
This gist shows the resulting debuginfo for a simple example. Here's a snippet:
The file and line have been added here. The line currently points to the beginning of the statement containing the yield (or await), because that's what the MIR source info points to for the yield terminator. (We may want to point to the yield or await line specifically, but that can be done independently of this change.)
Debuggers don't know how to use this kind of info yet. However, we're hoping to experiment with adding such support to Fuchsia's debugger. It would be exciting if someone were interested in adding similar to support to gdb/lldb.
r? @oli-obk
cc @eddyb @jonas-schievink
Part of #73524.