debuginfo: give unique names to closure and generator types #63875
Conversation
Closure types have been moved to the namespace where they are defined, and both closure and generator type names now include the disambiguator. This fixes an exception when lldb prints nested closures. Fixes rust-lang#57822
rust-highfive
added
the
S-waiting-on-review
|
Thanks for the PR, @philipc! I think this looks great and I'm OK with not adding the |
|
📌 Commit 61ff27a has been approved by |
bors
added
S-waiting-on-bors
debuginfo: give unique names to closure and generator types Closure types have been moved to the namespace where they are defined, and both closure and generator type names now include the disambiguator. This fixes an exception when lldb prints nested closures. Fixes #57822 I haven't included the `DW_AT_artificial` changes discussed in #57822 because they make the output worse IMO, but I can easily add these if still required. For example, for the new test case the output is now: ``` (lldb) p g (issue_57822::main::closure-1) $1 = closure-1(closure(1)) ``` but adding `DW_AT_artificial` changes this to: ``` (lldb) p g (issue_57822::main::closure-1) $0 = closure-1 { } ``` Note that nested generators didn't cause the exception. I haven't determined why, but I think it makes sense to add the disambiguator for them too. It feels like we still don't really understand why closures were causing an error though. r? @michaelwoerister
|
☀️ Test successful - checks-azure |
| output.push_str("closure"); | ||
| ty::Closure(def_id, ..) => { | ||
| output.push_str(&format!( | ||
| "closure-{}", |
There was a problem hiding this comment.
I would've suggested the same syntax as rustc-demangle, but this is probably fine.
This entire module seems a bit redundant though, but I don't think I have the time to refactor it post-ty::print.
There was a problem hiding this comment.
You could open an issue and give some mentors.
There was a problem hiding this comment.
Yeah but that would require me to actually look at what this code is doing.
There might already be an issue open for "things to clean up post-ty::print".
michaelwoerister
added
the
beta-nominated
|
Nominating for beta backporting. This should be a low risk change and will make LLDB users happy. |
michaelwoerister
added
the
T-compiler
nikomatsakis
added
the
beta-accepted
|
Reviewed by the compiler team: |
pietroalbini
removed
the
beta-nominated
[beta] Rollup backports Rolled up: * [beta] Utilize released stable over dev-static #64046 Cherry picked: * Update rust-installer to limit memory use #63984 * debuginfo: give unique names to closure and generator types #63875 * ci: move libc mirrors to the rust-lang-ci-mirrors bucket #63772 * Fix nested eager expansions in arguments of `format_args` #63717 r? @ghost
Closure types have been moved to the namespace where they
are defined, and both closure and generator type names now
include the disambiguator.
This fixes an exception when lldb prints nested closures.
Fixes #57822
I haven't included the
DW_AT_artificialchanges discussed in #57822 because they make the output worse IMO, but I can easily add these if still required. For example, for the new test case the output is now:but adding
DW_AT_artificialchanges this to:Note that nested generators didn't cause the exception. I haven't determined why, but I think it makes sense to add the disambiguator for them too. It feels like we still don't really understand why closures were causing an error though.
r? @michaelwoerister