Reject integer suffix when tuple indexing#59421
Conversation
|
Fixed indenting in the block, you can ignore the whitespace changes with https://github.com/rust-lang/rust/pull/59421/files?w=1 |
|
The message could probably be rephrased to sound less awkward, but r=me either way. |
I had done exactly the same thing 😄 |
|
@bors r=varkor |
|
📌 Commit 6ad77b0 has been approved by |
|
I was going to blame beginner contributors and diagnostic improvements for this regression, but it turned out it was me - 44acea4. |
|
@estebank |
|
@petrochenkov @estebank This is already r+ed, should we add the extra test cases in follow up PRs? (or do you want to r-?) |
|
@bors r- |
|
Added the requested changes. r? @petrochenkov, can you take a look to give final approval? For the case you asked the output is Which is why I think it shouldn't be added to the test, as it is not handled by this PR. |
I think what this PR is doing (accept suffixed literal, report a non-fatal error, continue) is a better alternative than "expected identifier, found |
|
I see. Fixed. |
|
Thanks! |
|
📌 Commit 8d1cc72 has been approved by |
…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor
| let d = c.1suffix; | ||
| //~^ ERROR suffixes on a tuple index are invalid | ||
| println!("{}", d); | ||
| let s = X { 0suffix: 0, 1: 1, 2: 2 }; |
There was a problem hiding this comment.
(don't r- the PR in either case since it's in a rollup now... if changes need to be made, do it in a follow up PR)
…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor
…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor
Rollup of 18 pull requests Successful merges: - #57293 (Make some lints incremental) - #57565 (syntax: Remove warning for unnecessary path disambiguators) - #58253 (librustc_driver => 2018) - #58837 (librustc_interface => 2018) - #59268 (Add suggestion to use `&*var` when `&str: From<String>` is expected) - #59283 (Make ASCII case conversions more than 4× faster) - #59284 (adjust MaybeUninit API to discussions) - #59372 (add rustfix-able suggestions to trim_{left,right} deprecations) - #59390 (Make `ptr::eq` documentation mention fat-pointer behavior) - #59393 (Refactor tuple comparison tests) - #59420 ([CI] record docker image info for reuse) - #59421 (Reject integer suffix when tuple indexing) - #59430 (Renames `EvalContext` to `InterpretCx`) - #59439 (Generalize diagnostic for `x = y` where `bool` is the expected type) - #59449 (fix: Make incremental artifact deletion more robust) - #59451 (Add `Default` to `std::alloc::System`) - #59459 (Add some tests) - #59460 (Include id in Thread's Debug implementation) Failed merges: r? @ghost
Reject invalid literal suffixes in tuple indexing, tuple struct indexing, and struct field name position Tracking issue: rust-lang#60210 Closes rust-lang#60210 ## Summary Bump the ["suffixes on a tuple index are invalid" non-lint pseudo future-incompatibility warning (rust-lang#60210)][issue-60210][^non-lint] to a **hard error** across all editions, rejecting the remaining carve outs from accidentally accepted invalid suffixes since Rust **1.27**. - We accidentally accepted invalid suffixes in tuple indexing positions in Rust **1.27**. Originally reported at rust-lang#59418. - We tried to hard reject all invalid suffixes in rust-lang#59421, but unfortunately it turns out there were proc macros accidentally relying on it: rust-lang#60138. - We temporarily accepted `{i,u}{32,size}` in rust-lang#60186 (the "*carve outs*") to mitigate *immediate* ecosystem impact, but it came with an FCW warning indicating that we wanted to reject it after a few Rust releases. - Now (1.89.0) is a few Rust releases later (1.35.0), thus I'm proposing to **also reject the carve outs**. - `std::mem::offset_of!` stabilized in Rust **1.77.0** happens to use the same "don't expect suffix" code path which has the carve outs, so it also accepted the carve out suffixes. I'm proposing to **reject this case as well**. ## What specifically breaks? Code that still relied on invalid `{i,u}{32,size}` suffixes being temporarily accepted by rust-lang#60186 as an ecosystem impact mitigation measure (cf. rust-lang#60138). Specifically, the following cases (particularly the construction of these forms in proc macros like reported in rust-lang#60138): ### Position 1: Invalid `{i,u}{32,size}` suffixes in tuple indexing ```rs fn main() { let _x = (42,).0invalid; // Already error, already rejected by rust-lang#59421 let _x = (42,).0i8; // Already error, not one of the rust-lang#60186 carve outs. let _x = (42,).0usize; // warning: suffixes on a tuple index are invalid } ``` ### Position 2: Invalid `{i,u}{32,size}` suffixes in tuple struct indexing ```rs fn main() { struct X(i32); let _x = X(42); let _x = _x.0invalid; // Already error, already rejected by rust-lang#59421 let _x = _x.0i8; // Already error, not one of the rust-lang#60186 carve outs. let _x = _x.0usize; // warning: suffixes on a tuple index are invalid } ``` ### Position 3: Invalid `{i,u}{32,size}` suffixes in numeric struct field names ```rs fn main() { struct X(i32, i32, i32); let _x = X(1, 2, 3); let _y = X { 0usize: 42, 1: 42, 2: 42 }; // warning: suffixes on a tuple index are invalid match _x { X { 0usize: 1, 1: 2, 2: 3 } => todo!(), // warning: suffixes on a tuple index are invalid _ => {} } } ``` ### Position 4: Invalid `{i,u}{32,size}` suffixes in `std::mem::offset_of!` While investigating the warning, unfortunately I noticed `std::mem::offset_of!` also happens to use the "expect no suffix" code path which had the carve outs. So this was accepted since Rust **1.77.0** with the same FCW: ```rs fn main() { #[repr(C)] pub struct Struct<T>(u8, T); assert_eq!(std::mem::offset_of!(Struct<u32>, 0usize), 0); //~^ WARN suffixes on a tuple index are invalid } ``` ### The above forms in proc macros For instance, constructions like (see tracking issue rust-lang#60210): ```rs let i = 0; quote! { foo.$i } ``` where the user needs to actually write ```rs let i = syn::Index::from(0); quote! { foo.$i } ``` ### Crater results Conducted a crater run (rust-lang#145463 (comment)). - https://github.com/AmlingPalantir/r4/tree/256af3c72f094b298cd442097ef7c571d8001f29: genuine regression; "invalid suffix `usize`" in derive macro. Has a ton of other build warnings, last updated 6 years ago. - Exactly the kind of intended breakage. Minimized down to https://github.com/AmlingPalantir/r4/blob/256af3c72f094b298cd442097ef7c571d8001f29/validates_derive/src/lib.rs#L71-L75, where when interpolation uses `quote`'s `ToTokens` on a `usize` index (i.e. on tuple struct `Tup(())`), the generated suffix becomes `.0usize` (cf. Position 2). - Notified crate author of breakage in AmlingPalantir/r4#1. - Other failures are unrelated or spurious. ## Review remarks - Commits 1-3 expands the test coverage to better reflect the current situation before doing any functional changes. - Commit 4 is an intentional **breaking change**. We bump the non-lint "suffixes on a tuple index are invalid" warning into a hard error. Thus, this will need a crater run and a T-lang FCP. ## Tasks - [x] Run crater to check if anyone is still relying on this being not a hard error. Determine degree of ecosystem breakage. - [x] If degree of breakage seems acceptable, draft nomination report for T-lang for FCP. - [x] Determine hard error on Edition 2024+, or on all editions. ## Accompanying Reference update - rust-lang/reference#1966 [^non-lint]: The FCW was implemented as a *non-lint* warning (meaning it has no associated lint name, and you can't `#![deny(..)]` it) because spans coming from proc macros could not be distinguished from regular field access. This warning was also intentionally impossible to silence. See rust-lang#60186 (comment). [issue-60210]: rust-lang#60210
Rollup merge of #145463 - jieyouxu:error-suffix, r=fmease Reject invalid literal suffixes in tuple indexing, tuple struct indexing, and struct field name position Tracking issue: #60210 Closes #60210 ## Summary Bump the ["suffixes on a tuple index are invalid" non-lint pseudo future-incompatibility warning (#60210)][issue-60210][^non-lint] to a **hard error** across all editions, rejecting the remaining carve outs from accidentally accepted invalid suffixes since Rust **1.27**. - We accidentally accepted invalid suffixes in tuple indexing positions in Rust **1.27**. Originally reported at #59418. - We tried to hard reject all invalid suffixes in #59421, but unfortunately it turns out there were proc macros accidentally relying on it: #60138. - We temporarily accepted `{i,u}{32,size}` in #60186 (the "*carve outs*") to mitigate *immediate* ecosystem impact, but it came with an FCW warning indicating that we wanted to reject it after a few Rust releases. - Now (1.89.0) is a few Rust releases later (1.35.0), thus I'm proposing to **also reject the carve outs**. - `std::mem::offset_of!` stabilized in Rust **1.77.0** happens to use the same "don't expect suffix" code path which has the carve outs, so it also accepted the carve out suffixes. I'm proposing to **reject this case as well**. ## What specifically breaks? Code that still relied on invalid `{i,u}{32,size}` suffixes being temporarily accepted by #60186 as an ecosystem impact mitigation measure (cf. #60138). Specifically, the following cases (particularly the construction of these forms in proc macros like reported in #60138): ### Position 1: Invalid `{i,u}{32,size}` suffixes in tuple indexing ```rs fn main() { let _x = (42,).0invalid; // Already error, already rejected by #59421 let _x = (42,).0i8; // Already error, not one of the #60186 carve outs. let _x = (42,).0usize; // warning: suffixes on a tuple index are invalid } ``` ### Position 2: Invalid `{i,u}{32,size}` suffixes in tuple struct indexing ```rs fn main() { struct X(i32); let _x = X(42); let _x = _x.0invalid; // Already error, already rejected by #59421 let _x = _x.0i8; // Already error, not one of the #60186 carve outs. let _x = _x.0usize; // warning: suffixes on a tuple index are invalid } ``` ### Position 3: Invalid `{i,u}{32,size}` suffixes in numeric struct field names ```rs fn main() { struct X(i32, i32, i32); let _x = X(1, 2, 3); let _y = X { 0usize: 42, 1: 42, 2: 42 }; // warning: suffixes on a tuple index are invalid match _x { X { 0usize: 1, 1: 2, 2: 3 } => todo!(), // warning: suffixes on a tuple index are invalid _ => {} } } ``` ### Position 4: Invalid `{i,u}{32,size}` suffixes in `std::mem::offset_of!` While investigating the warning, unfortunately I noticed `std::mem::offset_of!` also happens to use the "expect no suffix" code path which had the carve outs. So this was accepted since Rust **1.77.0** with the same FCW: ```rs fn main() { #[repr(C)] pub struct Struct<T>(u8, T); assert_eq!(std::mem::offset_of!(Struct<u32>, 0usize), 0); //~^ WARN suffixes on a tuple index are invalid } ``` ### The above forms in proc macros For instance, constructions like (see tracking issue #60210): ```rs let i = 0; quote! { foo.$i } ``` where the user needs to actually write ```rs let i = syn::Index::from(0); quote! { foo.$i } ``` ### Crater results Conducted a crater run (#145463 (comment)). - https://github.com/AmlingPalantir/r4/tree/256af3c72f094b298cd442097ef7c571d8001f29: genuine regression; "invalid suffix `usize`" in derive macro. Has a ton of other build warnings, last updated 6 years ago. - Exactly the kind of intended breakage. Minimized down to https://github.com/AmlingPalantir/r4/blob/256af3c72f094b298cd442097ef7c571d8001f29/validates_derive/src/lib.rs#L71-L75, where when interpolation uses `quote`'s `ToTokens` on a `usize` index (i.e. on tuple struct `Tup(())`), the generated suffix becomes `.0usize` (cf. Position 2). - Notified crate author of breakage in AmlingPalantir/r4#1. - Other failures are unrelated or spurious. ## Review remarks - Commits 1-3 expands the test coverage to better reflect the current situation before doing any functional changes. - Commit 4 is an intentional **breaking change**. We bump the non-lint "suffixes on a tuple index are invalid" warning into a hard error. Thus, this will need a crater run and a T-lang FCP. ## Tasks - [x] Run crater to check if anyone is still relying on this being not a hard error. Determine degree of ecosystem breakage. - [x] If degree of breakage seems acceptable, draft nomination report for T-lang for FCP. - [x] Determine hard error on Edition 2024+, or on all editions. ## Accompanying Reference update - rust-lang/reference#1966 [^non-lint]: The FCW was implemented as a *non-lint* warning (meaning it has no associated lint name, and you can't `#![deny(..)]` it) because spans coming from proc macros could not be distinguished from regular field access. This warning was also intentionally impossible to silence. See #60186 (comment). [issue-60210]: #60210
Reject invalid literal suffixes in tuple indexing, tuple struct indexing, and struct field name position Tracking issue: rust-lang/rust#60210 Closes rust-lang/rust#60210 ## Summary Bump the ["suffixes on a tuple index are invalid" non-lint pseudo future-incompatibility warning (#60210)][issue-60210][^non-lint] to a **hard error** across all editions, rejecting the remaining carve outs from accidentally accepted invalid suffixes since Rust **1.27**. - We accidentally accepted invalid suffixes in tuple indexing positions in Rust **1.27**. Originally reported at rust-lang/rust#59418. - We tried to hard reject all invalid suffixes in rust-lang/rust#59421, but unfortunately it turns out there were proc macros accidentally relying on it: rust-lang/rust#60138. - We temporarily accepted `{i,u}{32,size}` in rust-lang/rust#60186 (the "*carve outs*") to mitigate *immediate* ecosystem impact, but it came with an FCW warning indicating that we wanted to reject it after a few Rust releases. - Now (1.89.0) is a few Rust releases later (1.35.0), thus I'm proposing to **also reject the carve outs**. - `std::mem::offset_of!` stabilized in Rust **1.77.0** happens to use the same "don't expect suffix" code path which has the carve outs, so it also accepted the carve out suffixes. I'm proposing to **reject this case as well**. ## What specifically breaks? Code that still relied on invalid `{i,u}{32,size}` suffixes being temporarily accepted by rust-lang/rust#60186 as an ecosystem impact mitigation measure (cf. rust-lang/rust#60138). Specifically, the following cases (particularly the construction of these forms in proc macros like reported in rust-lang/rust#60138): ### Position 1: Invalid `{i,u}{32,size}` suffixes in tuple indexing ```rs fn main() { let _x = (42,).0invalid; // Already error, already rejected by #59421 let _x = (42,).0i8; // Already error, not one of the #60186 carve outs. let _x = (42,).0usize; // warning: suffixes on a tuple index are invalid } ``` ### Position 2: Invalid `{i,u}{32,size}` suffixes in tuple struct indexing ```rs fn main() { struct X(i32); let _x = X(42); let _x = _x.0invalid; // Already error, already rejected by #59421 let _x = _x.0i8; // Already error, not one of the #60186 carve outs. let _x = _x.0usize; // warning: suffixes on a tuple index are invalid } ``` ### Position 3: Invalid `{i,u}{32,size}` suffixes in numeric struct field names ```rs fn main() { struct X(i32, i32, i32); let _x = X(1, 2, 3); let _y = X { 0usize: 42, 1: 42, 2: 42 }; // warning: suffixes on a tuple index are invalid match _x { X { 0usize: 1, 1: 2, 2: 3 } => todo!(), // warning: suffixes on a tuple index are invalid _ => {} } } ``` ### Position 4: Invalid `{i,u}{32,size}` suffixes in `std::mem::offset_of!` While investigating the warning, unfortunately I noticed `std::mem::offset_of!` also happens to use the "expect no suffix" code path which had the carve outs. So this was accepted since Rust **1.77.0** with the same FCW: ```rs fn main() { #[repr(C)] pub struct Struct<T>(u8, T); assert_eq!(std::mem::offset_of!(Struct<u32>, 0usize), 0); //~^ WARN suffixes on a tuple index are invalid } ``` ### The above forms in proc macros For instance, constructions like (see tracking issue rust-lang/rust#60210): ```rs let i = 0; quote! { foo.$i } ``` where the user needs to actually write ```rs let i = syn::Index::from(0); quote! { foo.$i } ``` ### Crater results Conducted a crater run (rust-lang/rust#145463 (comment)). - https://github.com/AmlingPalantir/r4/tree/256af3c72f094b298cd442097ef7c571d8001f29: genuine regression; "invalid suffix `usize`" in derive macro. Has a ton of other build warnings, last updated 6 years ago. - Exactly the kind of intended breakage. Minimized down to https://github.com/AmlingPalantir/r4/blob/256af3c72f094b298cd442097ef7c571d8001f29/validates_derive/src/lib.rs#L71-L75, where when interpolation uses `quote`'s `ToTokens` on a `usize` index (i.e. on tuple struct `Tup(())`), the generated suffix becomes `.0usize` (cf. Position 2). - Notified crate author of breakage in AmlingPalantir/r4#1. - Other failures are unrelated or spurious. ## Review remarks - Commits 1-3 expands the test coverage to better reflect the current situation before doing any functional changes. - Commit 4 is an intentional **breaking change**. We bump the non-lint "suffixes on a tuple index are invalid" warning into a hard error. Thus, this will need a crater run and a T-lang FCP. ## Tasks - [x] Run crater to check if anyone is still relying on this being not a hard error. Determine degree of ecosystem breakage. - [x] If degree of breakage seems acceptable, draft nomination report for T-lang for FCP. - [x] Determine hard error on Edition 2024+, or on all editions. ## Accompanying Reference update - rust-lang/reference#1966 [^non-lint]: The FCW was implemented as a *non-lint* warning (meaning it has no associated lint name, and you can't `#![deny(..)]` it) because spans coming from proc macros could not be distinguished from regular field access. This warning was also intentionally impossible to silence. See rust-lang/rust#60186 (comment). [issue-60210]: rust-lang/rust#60210
Fix #59418.
r? @varkor