Don't show suggestion if slice pattern is not top-level by long-long-float · Pull Request #121236 · rust-lang/rust

long-long-float · 2024-02-17T15:55:23Z

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 · 2024-02-17T15:55:31Z

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

Nadrieril · 2024-02-17T16:19:43Z

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

Nadrieril · 2024-02-27T16:31:33Z

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 · 2024-03-02T10:11:21Z

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 · 2024-03-03T04:50:29Z

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

@rustbot ready

compiler/rustc_hir_typeck/src/pat.rs

Nadrieril · 2024-03-11T18:48:16Z

@rustbot author

compiler/rustc_hir_typeck/src/pat.rs

Nadrieril · 2024-03-17T08:34:18Z

Thank you!

@bors r+ rollup

bors · 2024-03-17T08:34:21Z

📌 Commit 04c966b has been approved by Nadrieril

It is now in the queue for this repository.

Nadrieril · 2024-03-17T08:34:49Z

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

@bors r-

long-long-float · 2024-03-17T10:23:23Z

I squashed all commits. Thank you for reviewing.

Nadrieril · 2024-03-17T12:53:19Z

👍

@bors r+ rollup

bors · 2024-03-17T12:53:22Z

📌 Commit 78e94cb has been approved by Nadrieril

It is now in the queue for this repository.

…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

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.

rustbot assigned Nadrieril Feb 17, 2024

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

This comment has been minimized.

Sign in to view

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 force-pushed the rust-fix-consider-slicing branch 2 times, most recently from b1a6410 to 9e5ade0 Compare February 27, 2024 14:47

long-long-float force-pushed the rust-fix-consider-slicing branch from 9e5ade0 to 300ca5c Compare March 3, 2024 04:36

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

This comment has been minimized.

Sign in to view

Nadrieril reviewed Mar 3, 2024

View reviewed changes

compiler/rustc_hir_typeck/src/pat.rs Outdated Show resolved Hide resolved

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

long-long-float force-pushed the rust-fix-consider-slicing branch from a74c295 to fe143a2 Compare March 16, 2024 13:47

Nadrieril reviewed Mar 16, 2024

View reviewed changes

compiler/rustc_hir_typeck/src/pat.rs Outdated Show resolved Hide resolved

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

Don't show suggestion if slice pattern is enclosed by any patterns

78e94cb

long-long-float force-pushed the rust-fix-consider-slicing branch from 04c966b to 78e94cb Compare March 17, 2024 10:22

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

matthiaskrgr mentioned this pull request Mar 17, 2024

Rollup of 3 pull requests #122637

Merged

bors merged commit 3ec2b7b into rust-lang:master Mar 17, 2024

rustbot added this to the 1.79.0 milestone Mar 17, 2024

Uh oh!

Conversation

long-long-float commented Feb 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 17, 2024

Uh oh!

This comment has been minimized.

Nadrieril commented Feb 17, 2024

Uh oh!

Nadrieril commented Feb 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

long-long-float commented Mar 2, 2024

Uh oh!

long-long-float commented Mar 3, 2024

Uh oh!

This comment has been minimized.

Uh oh!

Nadrieril commented Mar 11, 2024

Uh oh!

Uh oh!

Nadrieril commented Mar 17, 2024

Uh oh!

bors commented Mar 17, 2024

Uh oh!

Nadrieril commented Mar 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

long-long-float commented Mar 17, 2024

Uh oh!

Nadrieril commented Mar 17, 2024

Uh oh!

bors commented Mar 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

long-long-float commented Feb 17, 2024 •

edited

Loading

Nadrieril commented Feb 27, 2024 •

edited

Loading

Nadrieril commented Mar 17, 2024 •

edited

Loading