Reject integer suffix when tuple indexing #59421

estebank · 2019-03-25T23:11:47Z

estebank · 2019-03-25T23:12:59Z

Fixed indenting in the block, you can ignore the whitespace changes with https://github.com/rust-lang/rust/pull/59421/files?w=1

src/libsyntax/parse/parser.rs

varkor · 2019-03-25T23:16:05Z

The message could probably be rephrased to sound less awkward, but r=me either way.

varkor · 2019-03-25T23:16:57Z

Fixed indenting in the block

I had done exactly the same thing 😄

src/test/ui/parser/issue-59418.rs

estebank · 2019-03-26T04:39:24Z

@bors r=varkor

bors · 2019-03-26T04:39:25Z

📌 Commit 6ad77b0 has been approved by varkor

src/libsyntax/parse/parser.rs

petrochenkov · 2019-03-26T06:59:52Z

I was going to blame beginner contributors and diagnostic improvements for this regression, but it turned out it was me - 44acea4.

petrochenkov · 2019-03-26T07:31:21Z

@estebank
Could you also add test cases for suffixed literals in fields in struct expressions and struct patterns.

let s = S { 0suffix: 0 };
match s {
    S { 0suffix: _ } => {}
}

Centril · 2019-03-26T09:15:40Z

@petrochenkov @estebank This is already r+ed, should we add the extra test cases in follow up PRs? (or do you want to r-?)

estebank · 2019-03-26T16:53:29Z

@bors r-

estebank · 2019-03-26T17:20:06Z

Added the requested changes. r? @petrochenkov, can you take a look to give final approval?

For the case you asked the output is

error: expected identifier, found `0suffix`
  --> src/main.rs:12:17
   |
12 |     let s = X { 0suffix: 0, 1: 1, 2: 2 };
   |             -   ^^^^^^^ expected identifier
   |             |
   |             while parsing this struct

error: expected identifier, found `0suffix`
  --> src/main.rs:15:13
   |
15 |         X { 0suffix: _, .. } => {}
   |             ^^^^^^^ expected identifier

error[E0063]: missing field `0` in initializer of `X`
  --> src/main.rs:12:13
   |
12 |     let s = X { 0suffix: 0, 1: 1, 2: 2 };
   |             ^ missing `0`

error[E0027]: pattern does not mention fields `0`, `1`, `2`
  --> src/main.rs:15:9
   |
15 |         X { 0suffix: _, .. } => {}
   |         ^^^^^^^^^^^^^^^^^^^^ missing fields `0`, `1`, `2`
   |
   = note: trying to match a tuple variant with a struct variant pattern

Which is why I think it shouldn't be added to the test, as it is not handled by this PR.

src/libsyntax/parse/parser.rs

petrochenkov · 2019-03-26T18:41:40Z

@estebank

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 0suffix", so we can use it for struct expressions/patterns as well and unify the diagnostics.

…ield name

estebank · 2019-03-26T19:33:10Z

@petrochenkov

I see. Fixed.

petrochenkov · 2019-03-26T20:40:51Z

Thanks!
@bors r+

bors · 2019-03-26T20:40:52Z

📌 Commit 8d1cc72 has been approved by petrochenkov

@varkor

…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor

kpp · 2019-03-27T13:07:06Z

src/test/ui/parser/issue-59418.rs

+    let d = c.1suffix;
+    //~^ ERROR suffixes on a tuple index are invalid
+    println!("{}", d);
+    let s = X { 0suffix: 0, 1: 1, 2: 2 };


How about X { 0usize: 0, 1: 1, 2: 2 };?

(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)

@varkor

…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor

@varkor

…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor

@ghost

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 integer suffix when tuple indexing

91b7423

rust-highfive assigned varkor Mar 25, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2019

varkor reviewed Mar 25, 2019

View reviewed changes

src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved

kpp reviewed Mar 25, 2019

View reviewed changes

src/test/ui/parser/issue-59418.rs Show resolved Hide resolved

review comments

6ad77b0

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2019

petrochenkov reviewed Mar 26, 2019

View reviewed changes

src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 26, 2019

Use expect_no_suffix for error

c7ddb83

rust-highfive assigned petrochenkov and unassigned varkor Mar 26, 2019

varkor reviewed Mar 26, 2019

View reviewed changes

src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved

estebank added 2 commits March 26, 2019 12:09

Reword invalid suffixe errors

1bb3694

Add specific message for tuple struct invoked with suffixed numeric f…

8d1cc72

…ield name

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 26, 2019

Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2019

Rollup merge of rust-lang#59421 - estebank:tuple-index-suffix, r=petr…

892c18b

…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor

Centril mentioned this pull request Mar 27, 2019

Rollup of 13 pull requests #59457

Closed

kpp reviewed Mar 27, 2019

View reviewed changes

Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2019

Rollup merge of rust-lang#59421 - estebank:tuple-index-suffix, r=petr…

d3119e4

…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor

Centril mentioned this pull request Mar 27, 2019

Rollup of 17 pull requests #59466

Closed

cuviper added a commit to cuviper/rust that referenced this pull request Mar 28, 2019

Rollup merge of rust-lang#59421 - estebank:tuple-index-suffix, r=petr…

be34621

…ochenkov Reject integer suffix when tuple indexing Fix rust-lang#59418. r? @varkor

cuviper mentioned this pull request Mar 28, 2019

Rollup of 18 pull requests #59471

Merged

bors merged commit 8d1cc72 into rust-lang:master Mar 28, 2019

taiki-e mentioned this pull request Mar 30, 2019

Tuple indexing regression #59553

Closed

This was referenced Apr 1, 2019

(CRO-53) Balance tracking and synchronisation crypto-com/thaler#25

Merged

zeroize_derive: Build fails on latest nightly iqlusioninc/crates#172

Closed

estebank deleted the tuple-index-suffix branch November 9, 2023 05:19

jieyouxu mentioned this pull request Aug 16, 2025

Reject invalid literal suffixes in tuple indexing, tuple struct indexing, and struct field name position #145463

Merged

3 tasks

Reject integer suffix when tuple indexing #59421

Reject integer suffix when tuple indexing #59421

Uh oh!

Conversation

estebank commented Mar 25, 2019

Uh oh!

estebank commented Mar 25, 2019

Uh oh!

Uh oh!

varkor commented Mar 25, 2019

Uh oh!

varkor commented Mar 25, 2019

Uh oh!

Uh oh!

estebank commented Mar 26, 2019

Uh oh!

bors commented Mar 26, 2019

Uh oh!

Uh oh!

petrochenkov commented Mar 26, 2019

Uh oh!

petrochenkov commented Mar 26, 2019

Uh oh!

Centril commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estebank commented Mar 26, 2019

Uh oh!

estebank commented Mar 26, 2019

Uh oh!

Uh oh!

petrochenkov commented Mar 26, 2019

Uh oh!

estebank commented Mar 26, 2019

Uh oh!

petrochenkov commented Mar 26, 2019

Uh oh!

bors commented Mar 26, 2019

Uh oh!

kpp Mar 27, 2019

Choose a reason for hiding this comment

Uh oh!

Centril Mar 27, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Centril commented Mar 26, 2019 •

edited

Loading