-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Initial support for emitting DWARF for static variables #13143
Conversation
A quick glance seems to show there's no tests for structural types in statics (e.g. |
Oh, cool, I just missed it. (What about some non- |
Yeah the tests are quite repetitive and its not easy to pick out the details. I elected to omit the non- |
Please don't r+ before I had a chance to review this (although I don't expect any major issues, Craig has done great work so far) |
// debugger:rbreak zzz | ||
// debugger:run | ||
// debugger:finish | ||
// debugger:whatis 'basic-types-globals-metadata'::B |
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.
Could you put the '
after the B here and everywhere else? As it is here, it won't scale to nested modules[1] and these test files are often used as reference by users. We don't want to confuse anybody.
[1] http://osdir.com/ml/gdb.bugs.discuss/2002-11/msg00063.html
OK, I've taken a look at this. The implementation looks very good. I've added some comments to the test cases. Also, a test case with static enums would be a good idea. If you address the nits and add an enum test case, the PR has my blessing. It's strange that we always need to specify the variables' full path. With Clang and GDB specifying just the name works fine, but I couldn't find a significant difference in the DWARF generated for both. Maybe GDB's 'minimal language' is evaluated slightly different than C++. I looked into it more than an hour but couldn't solve the problem. But this should be of no concern for this PR. Excellent work! Thanks a lot for putting your time into this. |
Only supports crate level statics. No debug info is generated for function level statics. Closes rust-lang#9227.
Done. And yeah, it took a while to even figure you out that you needed to specify the full variable path. Enum test case is here https://github.com/mozilla/rust/pull/13143/files#diff-1f8000b73690bfcc4bed0d5bfc060628R1 to save you some scrolling. |
Looks good. You missed some single quotes in Please r+, ye rustic sages! |
@bors: retry |
Only supports crate level statics. No debug info is generated for function level statics. Closes #9227. As discussed at the end of the comments for #9227, I took an initial stab at adding support for function level statics and decided it would be enough work to warrant being split into a separate issue. See #13144 for the new issue describing the need to add support for function level static variables.
this is beautiful |
…onst-macro, r=Veykril fix: Insert whitespaces into static & const bodies if they are expanded from macro on hover Partially fixes rust-lang#13143. To resolve the other part we need to expand macros in unevaluated static & const bodies, and I'm not sure we want to. If for example it includes a call to `assert!()`, expanding it will lead to worse hover.
…tions, r=y21 Suggest `.cast`/`.cast_const`/`.cast_mut` in `transmute_ptr_as_ptr` Essentially pre-applies `ptr_as_ptr` - rust-lang/rust-clippy#6372 (comment) changelog: none
Only supports crate level statics. No debug info is generated for function level statics. Closes #9227.
As discussed at the end of the comments for #9227, I took an initial stab at adding support for function level statics and decided it would be enough work to warrant being split into a separate issue.
See #13144 for the new issue describing the need to add support for function level static variables.