Drastically restrict the grammar of tuple indices#154492
Drastically restrict the grammar of tuple indices#154492fmease wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Drastically restrict the grammar of tuple indices
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (53f0b05): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 11.4%, secondary -6.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 484.178s -> 486.044s (0.39%) |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
5072669 to
9fb994b
Compare
Not really, I still think 49718 was generally right, instead of introducing a separate lexical category for a corner case of the language we reuse something that already exists, and name resolution deals with the rest. |
|
I do understand where you're coming from. However, tuple indices are already blurry from a lexical vs. syntactical perspective due to us splitting float literals as you obviously know. I'm aware though that introspecting tokens like this in the parser is slightly unorthodox (well, just like float and punctuation splitting). I'll keep this in mind though. Still, I'd like to hear what T-lang thinks assuming crater comes back somewhat okay. If T-lang and you reject this idea, I'll accept that and I'll just resort to fixing (3):
|
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
|
@craterbot run mode=check-only crates=https://crater-reports.s3.amazonaws.com/pr-154492/retry-regressed-list.txt |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
…horthand, r=mu001999 Syntactically reject tuple index shorthands in struct patterns to fix a correctness regression Split out of PR rust-lang#154492. This fixes a correctness regression introduced in PR rust-lang#81235 from 2021. Crater was run in my other PR and didn't report any real regressions (rust-lang#154492 (comment)); a rerun has been issued for a few spurious builds (rust-lang#154492 (comment)) but I'm certain it won't find anything either. This is a theoretical breaking change that doesn't need any T-lang input IMHO since it's such a minute, niche and crystal clear bug that's not worth bothering them with (such a decision is not unprecedented). I'm adding it to the compatibility section of the release notes as is customary. The Reference doesn't need updating since it didn't adopt this bug and thus accurately describes this part of the grammar as it used to be before 2021-02-23 and as it's meant to be. The majority of the diff is doc comment additions & necessary UI test restructurings.
Rollup merge of #155698 - fmease:no-struct-pat-tuple-index-shorthand, r=mu001999 Syntactically reject tuple index shorthands in struct patterns to fix a correctness regression Split out of PR #154492. This fixes a correctness regression introduced in PR #81235 from 2021. Crater was run in my other PR and didn't report any real regressions (#154492 (comment)); a rerun has been issued for a few spurious builds (#154492 (comment)) but I'm certain it won't find anything either. This is a theoretical breaking change that doesn't need any T-lang input IMHO since it's such a minute, niche and crystal clear bug that's not worth bothering them with (such a decision is not unprecedented). I'm adding it to the compatibility section of the release notes as is customary. The Reference doesn't need updating since it didn't adopt this bug and thus accurately describes this part of the grammar as it used to be before 2021-02-23 and as it's meant to be. The majority of the diff is doc comment additions & necessary UI test restructurings.
|
☔ The latest upstream changes (presumably #155720) made this pull request unmergeable. Please resolve the merge conflicts. |
… r=mu001999 Syntactically reject tuple index shorthands in struct patterns to fix a correctness regression Split out of PR rust-lang/rust#154492. This fixes a correctness regression introduced in PR rust-lang/rust#81235 from 2021. Crater was run in my other PR and didn't report any real regressions (rust-lang/rust#154492 (comment)); a rerun has been issued for a few spurious builds (rust-lang/rust#154492 (comment)) but I'm certain it won't find anything either. This is a theoretical breaking change that doesn't need any T-lang input IMHO since it's such a minute, niche and crystal clear bug that's not worth bothering them with (such a decision is not unprecedented). I'm adding it to the compatibility section of the release notes as is customary. The Reference doesn't need updating since it didn't adopt this bug and thus accurately describes this part of the grammar as it used to be before 2021-02-23 and as it's meant to be. The majority of the diff is doc comment additions & necessary UI test restructurings.
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
Syntactically speaking, on stable & main,
[0-9][0-9_]*|0b[01][01_]*|0o[0-7][0-7_]*|0x[0-9a-fA-F][0-9a-fA-F_]*X { 00: x, 1_0: y, 0xF: z }X { 1e2: () },X { 1e+2: () }andX { 1suffix: () }offset_ofexprs andfield_oftypes are either unsuffixed integer literals or a subset of unsuffixed float literals[0-9][0-9_]*([eE]_*[0-9][0-9_]*)|0b[01][01_]*|0o[0-7][0-7_]*|0x[0-9a-fA-F][0-9a-fA-F_]*x.00,x.1_0,x.0xFandx.1e2x.1e+2andx.0suffixtuple index shorthands likeS { 0 }are legal in patterns but not in expressionsthey were made grammatical in PR RUST-81235 (2021) w/o lang FCP in order to improve diagnosticsclearly, they should not be syntactically legalI find this part of the grammar a bit unfortunate and I'd like to drastically restrict it to just
0|[1-9][0-9]*(i.e., plain non-negative decimal numbers without leading zeroes) with this PR. I hope it's obvious why that's desirable.Almost decade ago (still way after 1.0) we actually used to reject numeric bases, underscores and leading zeroes in tuple indices (leading zeroes specifically since PR RUST-47084 (2017)) but then that was undone by PR RUST-49718 (2018) w/o lang FCP in order to simplify the compiler. Moreover, we've been rejecting suffixes on them since at least 1.01.
Of course in Rust tuple indices aren't first class citizens and are closer to alphanumeric identifiers than proper natural numbers. Still, they're clearly meant to mimic natural numbers (or at least they used to). That's why I find it odd that
00is legal and unequal to0, similarly with1_000and1000. By rejecting leading zeroes and underscores we can "keep up the appearance that these indices are natural numbers instead of identifiers" which I quite like.Footnotes
With some hiccups, see RUST-59418, RUST-60186 and RUST-145463. ↩