[deubginfo] Add .debug_functions section #2474
Conversation
djolertrk
force-pushed
the
pr/debug-functions-section
branch
2 times, most recently
from
bb9a4e3 to
7fce755
Compare
djolertrk
force-pushed
the
pr/debug-functions-section
branch
from
7fce755 to
44db8ca
Compare
|
NOTE: The generation of this section depends on @bitwalker's work on assembler. |
bitwalker
left a comment
bitwalker
left a comment
There was a problem hiding this comment.
Just one question before I finish up my review, see the comment I left on debug_functions.rs
djolertrk
force-pushed
the
pr/debug-functions-section
branch
from
44db8ca to
2445f3e
Compare
There was a problem hiding this comment.
I'd just remove this for now
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a few comments inline - but these should be straight-forward to address.
Also, let's add a changelog entry for this PR.
| mast_forest.debug_info.clear_procedure_names(); | ||
| mast_forest.debug_info.extend_procedure_names(procedure_names); |
There was a problem hiding this comment.
I find the need to clear procedure names and then extend them a bit confusing - but I guess we do the same for error codes. Nothing to do in this PR, but maybe it is something we can remedy when we are refactoring MAST serialization (cc @huitseeker).
| assert!( | ||
| self.nodes.iter().any(|node| node.digest() == digest), | ||
| "attempted to insert procedure name for digest not in forest" | ||
| ); |
There was a problem hiding this comment.
I think here we'd want to check only against procedure roots - not all nodes. We could use something like find_procedure_root() method to check if a given digest is a procedure root.
There was a problem hiding this comment.
A key detail here is that these procedure names map to both exported and non-exported procedures in the forest, so as long as the forest provides a way to determine if a given node is a procedure root for a non-exported procedure, then 👍.
There was a problem hiding this comment.
Yes, I believe procedure_roots contains roots of all procedures (exported and not-exported).
djolertrk
force-pushed
the
pr/debug-functions-section
branch
from
c803e63 to
33d384e
Compare
|
Rebased on top of trunk and addressed the comments. |
This section contains the names of all procedures (both exported and private) in a package, keyed by their MAST root digest. This allows debuggers to resolve human-readable procedure names during execution.
- assert that the digest exists in the forest before inserting - minor cleanup
djolertrk
force-pushed
the
pr/debug-functions-section
branch
from
33d384e to
afa9840
Compare
djolertrk
force-pushed
the
pr/debug-functions-section
branch
from
afa9840 to
e8a7db5
Compare
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
All looks good! Thank you!
3 participants
This section contains the names of all procedures (both exported and private) in a package, keyed by their MAST root digest. This allows debuggers to resolve human-readable procedure names during execution.
TODO: Generate this section from asembly and compiler.