debug: don't set end_location at function exit #4972
Conversation
asterite
added
kind:bug
spec/std/callstack_spec.cr
Outdated
|
|
||
| # The first line is the old (incorrect) behaviour, | ||
| # the second line is the new (correct) behaviour. | ||
| # TODO: keep only the second one after Crystal 0.23.1 |
There was a problem hiding this comment.
Perhaps we could use the crystal compiler programatically to ensure we don't have to handle multiple compiler versions, just master.
There was a problem hiding this comment.
It's a good idea. I'll change it later.
There was a problem hiding this comment.
Actually, I'd prefer now to do that just now. That means the spec will depend on the compiler, and that will slow down std_spec a lot.
|
This looks like a great fix, but regarding specs I think we should do something like #4718 sooner or later. I think sticking all debuginfo specs inside "callstack" is a bad idea. |
|
@RX14 I agree. I just fixed the spec because it was failing. It's just coincidence that there's a spec for this (I'm sure it's testing the callstack, not the debug info location) |
|
Yes, the spec is testing that the DWARF information (function name, file:line:colum) is correctly parsed and printed, so we don't have regressions, and we get a failure when porting to an unsupported system (can't parse executable, failed to find/parse DWARF). |
The codegen was setting the end location of a method for the last
returnLLVM instruction.So for example for this code:
we would get:
foo.cr at line 3 is the
end, but it should be foo.cr at line 2,bar.The same goes for indexable.cr at line 74, it points to the
endwhere it should point to the last expression (one line above it).The solution is to leave the debug info for the last
returnLLVM instruction as the location of the previously executed expression.The indexable.cr line 0 is still a mistery, but it must be an unrelated issue.