Initial implementation of debuginfo in MIR trans. #32803
Conversation
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
|
See my comment on #31005 for some explanations related to the debuginfo tests which are still not passing. |
| let debug_loc = DebugLoc::ScopeAt(self.scopes[terminator.scope.index()], | ||
| terminator.span); | ||
| debug_loc.apply_bcx(&bcx); | ||
| debug_loc.apply(bcx.fcx()); |
|
🔑 |
| landingpad.at_start(|bcx| for op in args { | ||
| self.set_operand_dropped(bcx, op); | ||
| landingpad.at_start(|bcx| { | ||
| debug_loc.apply_bcx(bcx); |
There was a problem hiding this comment.
This method seriously needs a better name. debug_loc.apply_to(bcx) or debug_loc.apply_to_bcx(bcx).
| let debug_loc = DebugLoc::ScopeAt(self.scopes[terminator.scope.index()], | ||
| terminator.span); | ||
| debug_loc.apply_to_bcx(&bcx); | ||
| debug_loc.apply(bcx.fcx()); |
There was a problem hiding this comment.
Sadly, old trans still exists and every with_block call makes it show up.
|
Solid. |
| } | ||
| }; | ||
|
|
||
| unsafe { | ||
| llvm::LLVMSetCurrentDebugLocation(cx.raw_builder(), metadata_node); | ||
| if builder.is_none() { |
There was a problem hiding this comment.
Why is it necessary to make this conditional? It seems to me that if this condition is removed, then in MIR we can avoid doing both apply_to_bcx and apply in certain cases (only apply_to_bcx would be enough)
There was a problem hiding this comment.
They work on different builders (global, i.e. "old", vs MIR). Without this condition, what would happen is that it would not bother setting the location in different MIR builders because it matched the last location (but the last location was set on a different builder).
|
☔ The latest upstream changes (presumably #32738) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r+ |
|
📌 Commit 373b6ec has been approved by |
|
LGTM! |
Progress is made towards #31005, but several issues remain, such as #32790.