-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Debug info #7134
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
Debug info #7134
Conversation
Awesome work, though is there are good reason you are using an |
struct Metadata<T> { | ||
node: ValueRef, | ||
data: T | ||
#[inline(always)] |
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.
Does this really need to be always inlined? I'd prefer you let LLVM decide here.
It probably doesn't matter here but over-use of #[inline(always)]
is a bit of a problem in general that I'm trying to prevent.
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.
This is a fairly often used function, so it'd better be inlined. But if you vouch for LLVM optimizer, I have no problem removing that.
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.
The optimizer does a pretty good job overall, with only a few cases (notably iterators) requiring a forced inline. Even just #[inline]
would likely be enough.
I need DebugContext to be mutable, and if I don't make it |
Just a request to hold off acting on this PR until I have a chance to look through it. I'm getting on a plane or I'd do so now. |
@Aatch: I can wait, though you guys have an intern starting on Monday, who's supposed to work on debug info. I wanted to land this before he gets in. |
This is a delightful surprise! Thanks @vadimcn. |
@vadimcn the PR went through, so you can rebase now, |
Sorry about the comments that have already been addressed in subsequent commits. |
@jdm: Thanks for the review, but it looks like you are reviewing only the first commit of my PR. Some comments still apply, but a lot of this stuff had been changed in later commits. |
Great work! I appreciate your efforts to get this done in time for Michael's first week on the project, too :) I'm happy for this to go in the tree after a rebase; my comments could be addressed in a subsequent PR. |
Disabled create_arg
Misc refactoring.
Fixed whitespace "errors".
Made debugger scripts source line insensitive.
Ok, rebase is done. |
This commit fixes rustc's debug info generation and turns debug-info tests back on. The old generator used to write out LLVM metadata directly, however it seems that debug metadata format is not stable and keeps changing from release to release. So I wrapped LLVM's official debug info API - the DIBuilder class, and now rustc will use that. One bit of old functionality that still doesn't work, is debug info for function arguments. Someone more familiar with the compiler guts will need to look into that. Also, unfortunately, debug info is still won't work on Windows,- due to a LLVM bug (http://llvm.org/bugs/show_bug.cgi?id=16249). Resolves issues #5836, #5848, #6814
@michaelwoerister: yes, please go ahead and take it over. Have fun! |
@vadimcn Thanks again! The code looks really good. |
Finish MSRV for cloned_instead_of_copied changelog: none r? `@Manishearth`
This commit fixes rustc's debug info generation and turns debug-info tests back on.
The old generator used to write out LLVM metadata directly, however it seems that debug metadata format is not stable and keeps changing from release to release. So I wrapped LLVM's official debug info API - the DIBuilder class, and now rustc will use that.
One bit of old functionality that still doesn't work, is debug info for function arguments. Someone more familiar with the compiler guts will need to look into that.
Also, unfortunately, debug info is still won't work on Windows,- due to a LLVM bug (http://llvm.org/bugs/show_bug.cgi?id=16249).
Resolves issues #5836, #5848, #6814