-
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
Omit DW_AT_linkage_name when it is the same as DW_AT_name #72620
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit d0db4203b0077e739519ebd1354c08e15e05c8ee with merge db80f4e965b55dc1563d5f08e9023e8eae736dbb... |
☀️ Try build successful - checks-azure |
Queued db80f4e965b55dc1563d5f08e9023e8eae736dbb with parent 783139b, future comparison URL. |
Finished benchmarking try commit db80f4e965b55dc1563d5f08e9023e8eae736dbb, comparison URL. |
// When empty, linkage_name field is omitted, | ||
// which is what we want for no_mangle statics | ||
let linkage_name = linkage_name.as_deref().unwrap_or(""); | ||
let linkage_name = if &var_name == &linkage_name { "" } else { linkage_name }; |
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.
Nit: var_name and linkage_name should both have type &str here, so why the extra & on both?
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.
var_name is SymbolStr, but having '&' on both is indeed unnecessary.
The DWARF standard suggests that it might be useful to include `DW_AT_linkage_name` when it is *distinct* from the identifier name.
This looks reasonable to me, and semantically correct, if a compiler team member would like to sign off on the implementation. |
r? @eddyb |
@bors r+ |
📌 Commit e4b7d2c has been approved by |
…arth Rollup of 13 pull requests Successful merges: - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name) - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results) - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans) - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.) - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround) - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm) - rust-lang#73597 (Record span of `const` kw in GenericParamKind) - rust-lang#73629 (Make AssocOp Copy) - rust-lang#73681 (Update Chalk to 0.14) - rust-lang#73707 (Fix links in `SliceIndex` documentation) - rust-lang#73719 (emitter: column width defaults to 140) - rust-lang#73729 (disable collectionbenches for android) - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`) Failed merges: r? @ghost
…arth Rollup of 13 pull requests Successful merges: - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name) - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results) - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans) - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.) - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround) - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm) - rust-lang#73597 (Record span of `const` kw in GenericParamKind) - rust-lang#73629 (Make AssocOp Copy) - rust-lang#73681 (Update Chalk to 0.14) - rust-lang#73707 (Fix links in `SliceIndex` documentation) - rust-lang#73719 (emitter: column width defaults to 140) - rust-lang#73729 (disable collectionbenches for android) - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`) Failed merges: r? @ghost
The DWARF standard suggests that it might be useful to include
DW_AT_linkage_name
when it is distinct from the identifier name.Fixes #46487.
Fixes #59422.