Create proper debug info for functions and function pointers #27025
Conversation
|
This needs more work, closure DI is still wrong |
dotdash
force-pushed
the
issue-26484
branch
2 times, most recently
from
b2b3827 to
3629f00
Compare
| @@ -796,12 +796,31 @@ pub fn type_metadata<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, | |||
| } | |||
| } | |||
| ty::TyBareFn(_, ref barefnty) => { | |||
| subroutine_type_metadata(cx, unique_type_id, &barefnty.sig, usage_site_span) | |||
| let fn_metadata = subroutine_type_metadata(cx, | |||
There was a problem hiding this comment.
any reason that fn_metadata is defined up here but is only used after the match below? (I note this largely because of that first match arm that returns, so I would have expected this let to come below the match)
There was a problem hiding this comment.
The match checks whether subroutine_type_metadata happened to already create the metadata that we're trying to create, i.e. whether it recursively called type_metadata for this type again.
Thinking about it, I feel like this isn't actually possible for function types, so the match could be dropped. Is that correct?
|
I don't know how to come up with a smaller test case, but I could add a check that checks for the debugger type output for function pointers. We had: And now we have: Would that be enough? |
|
Removed that match and added a basic test for function pointer debug info. |
|
Hm, this currently triggers an assertion in LLVM when running the debuginfo tests. Will investigate... |
|
@dotdash here is my reduction of the original https://gist.github.com/pnkfelix/268609f5a2e88cddac63 It is a very strange twisty test. I tried to remove parts that seemed unnecessary, but that tended to cause the bug to disappear. |
|
In general this looks very good to me. Thanks a lot for looking into this @dotdash! |
|
☔ The latest upstream changes (presumably #27076) made this pull request unmergeable. Please resolve the merge conflicts. |
We're currently using the actual function type as the return type when creating the debug info for a function, so we're actually creating debug info for a function that takes the same parameters, and returns the actual function type, which is completely wrong.
Internally, the arguments passed to the closure are represented by a tuple, but the actual function takes them as individual arguments, so we have to untuple the arguments before creating the debuginfo.
Instead of generating pointer debug info, we're currently generating subroutine debug info.
|
OK, so I restored the struct S {
f: fn(&S)
}I also created a minimal testcase from the reduced one that @pnkfelix has provided. The assertion I have been hitting is the same as in #26447 and the testsuite only triggers it with debug info and optimizations enabled. So since this is an existing bug and we don't have build bots for that setup yet, I think this should land even with that bug still left in. |
|
Yeah okay, at least this will get us to the point where we can bootstrap |
|
Based on @michaelwoerister positive feedback above and @dotdash's note that the assertion is likely a preexisting bug, I'm r+'ing this. |
|
@bors r+ 9a14f80 |
|
⌛ Testing commit 9a14f80 with merge 9a31543... |
|
💔 Test failed - auto-mac-64-opt |
|
gah, I forgot that a _run_pass test is actually executed by the testsuite :-/ |
|
@bors r=pnkfelix 955690f |
|
⌛ Testing commit 955690f with merge bbd42a0... |
|
💔 Test failed - auto-linux-64-x-android-t |
Variables for closures hold a tuple of captured variables, and not the function itself. Fixes rust-lang#26484
See the commits for details r? @arielb1
See the commits for details
r? @arielb1