-
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
Temporarily accept [i|u][32|size] suffixes on a tuple index and warn #60186
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
I tried checking for only spans coming through the macro system, but because the error is pointing at the span for the proc macro itself, it is a regular span and we have no way to differentiate from regular field access. The warning is unsilenceable on purpose. |
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.
r=me if we link to the new issue I just filed
); | ||
err.note(&format!( | ||
"`{}` is *temporarily* accepted on tuple index fields as it was \ | ||
incorrectly accepted on stable for a few releases", |
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 affects my opinion, by the way -- what is the range of time in which it was accepted? I was assuming it has been accepted forever.
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.
Bisected (using godbolt) to 1.27 being the point at which it was allowed;
Test program:
fn main() {
let x = (0,);
x.0usize;
}
err.help( | ||
"on proc macros, you'll want to use `syn::Index::from` or \ | ||
`proc_macro::Literal::*_unsuffixed` for code that will desugar \ | ||
to tuple field access", |
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.
👍 to giving some explicit help here!
@bors r=nikomatsakis |
📌 Commit 4c01573 has been approved by |
Temporarily accept [i|u][32|size] suffixes on a tuple index and warn Fix rust-lang#60138. rust-lang#59553 will need to be kept open to track the change back to rejecting this code a few versions down thee line.
Temporarily accept [i|u][32|size] suffixes on a tuple index and warn Fix rust-lang#60138. rust-lang#59553 will need to be kept open to track the change back to rejecting this code a few versions down thee line.
Rollup of 5 pull requests Successful merges: - #56278 (Future-proof MIR for dedicated debuginfo.) - #59739 (Stabilize futures_api) - #59822 (Fix dark css rule) - #60186 (Temporarily accept [i|u][32|size] suffixes on a tuple index and warn) - #60190 (Don't generate unnecessary rmeta files.) Failed merges: r? @ghost
[beta] Rollup backports Cherry-picked: * #59886: musl: do not compress debug section * #59891: Fix the link to sort_by_cached_key * #59911: Revert "compile crates under test w/ -Zemit-stack-sizes" * #59978: rustdoc: Remove default keyword from re-exported trait methods * #59989: Fix links to Atomic* in RELEASES.md * #60186: Temporarily accept [i|u][32|size] suffixes on a tuple index and warn * #60309: Add 1.34.1 release notes Rolled up: * #60273: [beta] bootstrap; remove redundant imports. r? @ghost
Fix #60138.
#59553 will need to be kept open to track the change back to rejecting this code a few versions down thee line.