-
Notifications
You must be signed in to change notification settings - Fork 77
feat(debuginfo): Add support for debuginfo, without scope support #455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
TODO: 1. Add int.rs locations 2. Add demangling support 3. Add debug scope support 4. Add vtable support 5. Clean up builder.rs locations
TODO: 1. Clean the unnecessary locations in builder.rs & int.rs 2. Add demangling support 3. Add debug scope support 4. Add vtable support 5. Clean up builder.rs locations
TODO:
|
Thank you so much! I can't wait to try this. I'm perfectly fine with merging this PR as is, but I was wondering if you knew why rustc doesn't handle the debuginfo in the |
Short AnswerI guess I knew it. In fact, it was my initial idea of applying the debuginfo. WarningThis version of debuginfo emission looks miserable now, don't expect too much, and this is only a proof of concept/"draft" at this time. ExperimentThis experiment is to show that no GCC debug location is to be generated, if You can simply put some My GuessesRustc relies on the per-function Since the generation of function debug context is banned on the first line by the It seems, though I have no evidence to this, that an implementation with debuginfo applied to the target afterwards relies on the scope to be implemented. Even if that method is our final destination(And I do believe it will be, as long as the cg_gcc can last longer and better than gccrs.), we are still in need of more modifications to the gccjit. I do have a private fork of that gccjit version in my computer, but having discovered that debuginfo generation can be accomplished without too much modification to gccjit, using the combination you see, I decided to create such an experimental version of debug info. Further WorkWhether we should use the update approach or the non-standalone approach, we will always need the scope support. Currently, this is scheduled after the |
However, I do mean to have it tested using the CI since there are always suprises. |
To set the debuginfo afterwards, we first have to read the mapping from the debuginfo to the object from a separate function, this has a similar overhead to doing it in builder.rs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending what I reviewed for now.
fe96113
to
6170f48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! (Sorry for all the nitpicks.)
Co-authored-by: antoyo <[email protected]>
Co-authored-by: antoyo <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more nits.
src/debuginfo.rs
Outdated
if ! self.sess().target.is_like_msvc { | ||
col } else { | ||
UNKNOWN_COLUMN_NUMBER | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the difference compared with the code from cg_llvm. It just seems to me that the condition is_like_msvc
was moved from after the match
to inside it.
And that the return type was changed from a struct to a tuple containing the same information. Could you please explain to me how this is different?
1. Revert to the original `lookup_debug_loc` of DebugLoc return type 2. Removed the commented code of scope lookup
This PR depends on rust-lang/gccjit.rs#28.