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

Don't show suggestion if slice pattern is not top-level #121236

Merged
merged 1 commit into from Mar 17, 2024

Conversation

long-long-float
Copy link
Contributor

@long-long-float long-long-float commented Feb 17, 2024

Close #120605

Don't show suggestion to add slicing ([..]) if the slice pattern is enclosed by struct like Struct { a: [] }.

For example, current rustc makes a suggestion as a comment. However, the pattern a: [] is wrong, not scrutinee &self.a.
In this case, the structure type a: Vec<Struct> and the pattern a: [] are different so I think the pattern should be fixed, not the scrutinee.
If the parent of the pattern that was the target of the error is a structure, I made the compiler not show a suggestion.

pub struct Struct {
    a: Vec<Struct>,
}

impl Struct {
    pub fn test(&self) {
        if let [Struct { a: [] }] = &self.a {
//             ^^^^^^^^^^^^^^^^^^   ------- help: consider slicing here: `&self.a[..]`
            println!("matches!")   
        }
    }
}

Note:

  • I created PatInfo.history to store parent-child relationships for patterns, but this may be inefficient.
    • I use two fields parent_kind and current_kind instead of vec. It may not performance issue.
  • Currently only looking at direct parents, but may need to look at deeper ancestry.

@rustbot
Copy link
Collaborator

rustbot commented Feb 17, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 17, 2024
@rust-log-analyzer

This comment has been minimized.

@Nadrieril
Copy link
Contributor

Hi! Thanks for your contribution.

Let me know with @rustbot ready when you've found a solution that doesn't overflow or if you need help.

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 17, 2024
@long-long-float long-long-float force-pushed the rust-fix-consider-slicing branch 2 times, most recently from b1a6410 to 9e5ade0 Compare February 27, 2024 14:47
@Nadrieril
Copy link
Contributor

Nadrieril commented Feb 27, 2024

My first question is: the example you show gives a correct suggestion: to match on self.a with a slice pattern you really do need to use self.a[..] or self.a.as_slice(). This is different from #120605.

Could you add two tests: one with the example you mention, and one with the example in #120605 so we can see how your code handles them?

@long-long-float
Copy link
Contributor Author

The reason why I use self.a without [..] is that the errors displayed are the same in the code with and without [..].
The following example shows that there is no suggestion for the error for test.rs:8:29.
Note that the error for test.rs:8:16 was added while resolving this issue (I think this error and suggestion is correct).

# self.a without [..]
-> % rustc +stage1 test.rs
error[E0529]: expected an array or slice, found `Vec<Struct>`
 --> test.rs:8:16
  |
8 |         if let [Struct { a: [] }] = &self.a {
  |                ^^^^^^^^^^^^^^^^^^   ------- help: consider slicing here: `&self.a[..]`
  |                |
  |                pattern cannot match with input type `Vec<Struct>`

error[E0529]: expected an array or slice, found `Vec<Struct>`
 --> test.rs:8:29
  |
8 |         if let [Struct { a: [] }] = &self.a {
  |                             ^^ pattern cannot match with input type `Vec<Struct>`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0529`.
# self.a with [..]
-> % rustc +stage1 test.rs
error[E0529]: expected an array or slice, found `Vec<Struct>`
 --> test.rs:8:29
  |
8 |         if let [Struct { a: [] }] = &self.a[..] {
  |                             ^^ pattern cannot match with input type `Vec<Struct>`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0529`.

Could you add two tests: one with the example you mention, and one with the example in #120605 so we can see how your code handles them?

Yes, I will add them today.

@long-long-float
Copy link
Contributor Author

I added a test for the issue. Could you review?

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 3, 2024
@rust-log-analyzer

This comment has been minimized.

@Nadrieril
Copy link
Contributor

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2024
@Nadrieril
Copy link
Contributor

Thank you!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 17, 2024

📌 Commit 04c966b has been approved by Nadrieril

It is now in the queue for this repository.

@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 17, 2024
@Nadrieril
Copy link
Contributor

Nadrieril commented Mar 17, 2024

Oops wait, could you squash all commits into one please?

@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 17, 2024
@Nadrieril Nadrieril changed the title Don't show suggestion if slice pattern is enclosed by struct Don't show suggestion if slice pattern is not top-level Mar 17, 2024
@long-long-float
Copy link
Contributor Author

I squashed all commits. Thank you for reviewing.

@Nadrieril
Copy link
Contributor

👍

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 17, 2024

📌 Commit 78e94cb has been approved by Nadrieril

It is now in the queue for this repository.

@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 17, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2024
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#121236 (Don't show suggestion if slice pattern is not top-level)
 - rust-lang#121787 (run change tracker even when config parse fails)
 - rust-lang#122633 (avoid unnecessary collect())

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3ec2b7b into rust-lang:master Mar 17, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2024
Rollup merge of rust-lang#121236 - long-long-float:rust-fix-consider-slicing, r=Nadrieril

Don't show suggestion if slice pattern is not top-level

Close rust-lang#120605

Don't show suggestion to add slicing (`[..]`) if the slice pattern is enclosed by struct like `Struct { a: [] }`.

For example, current rustc makes a suggestion as a comment. However, the pattern `a: []` is wrong, not scrutinee `&self.a`.
In this case, the structure type `a: Vec<Struct>` and the pattern `a: []` are different so I think the pattern should be fixed, not the scrutinee.
If the parent of the pattern that was the target of the error is a structure, I made the compiler not show a suggestion.

```rs
pub struct Struct {
    a: Vec<Struct>,
}

impl Struct {
    pub fn test(&self) {
        if let [Struct { a: [] }] = &self.a {
//             ^^^^^^^^^^^^^^^^^^   ------- help: consider slicing here: `&self.a[..]`
            println!("matches!")
        }
    }
}
```

Note:

* ~~I created `PatInfo.history` to store parent-child relationships for patterns, but this may be inefficient.~~
  * I use two fields `parent_kind` and `current_kind` instead of vec. It may not performance issue.
* Currently only looking at direct parents, but may need to look at deeper ancestry.
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect "consider slicing" suggestion with Vec inside a struct
5 participants