Skip to content
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

Reject integer suffix when tuple indexing #59421

Merged
merged 5 commits into from
Mar 28, 2019

Conversation

estebank
Copy link
Contributor

Fix #59418.

r? @varkor

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

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

@varkor
Copy link
Member

varkor commented Mar 25, 2019

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

@varkor
Copy link
Member

varkor commented Mar 25, 2019

Fixed indenting in the block

I had done exactly the same thing 😄

@estebank
Copy link
Contributor Author

@bors r=varkor

@bors
Copy link
Contributor

bors commented Mar 26, 2019

📌 Commit 6ad77b0 has been approved by varkor

@bors 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
Copy link
Contributor

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

@petrochenkov
Copy link
Contributor

@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
Copy link
Contributor

Centril commented Mar 26, 2019

@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
Copy link
Contributor Author

@bors r-

@bors 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
@estebank
Copy link
Contributor Author

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.

@rust-highfive rust-highfive assigned petrochenkov and unassigned varkor Mar 26, 2019
@petrochenkov
Copy link
Contributor

@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.

@estebank
Copy link
Contributor Author

@petrochenkov

I see. Fixed.

@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 26, 2019

📌 Commit 8d1cc72 has been approved by petrochenkov

@bors 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
…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 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Reject integer suffix when tuple indexing

Fix rust-lang#59418.

r? @varkor
cuviper added a commit to cuviper/rust that referenced this pull request Mar 28, 2019
…ochenkov

Reject integer suffix when tuple indexing

Fix rust-lang#59418.

r? @varkor
bors added a commit that referenced this pull request Mar 28, 2019
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
@bors bors merged commit 8d1cc72 into rust-lang:master Mar 28, 2019
@estebank estebank deleted the tuple-index-suffix branch November 9, 2023 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants