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

Better diagnostics for '..' pattern fragment not in the last position #49268

Merged
merged 3 commits into from
Mar 24, 2018

Conversation

ordovicia
Copy link
Contributor

Fixes #49257.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eddyb (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2018
@petrochenkov petrochenkov assigned petrochenkov and unassigned eddyb Mar 22, 2018
let p = Point { x: 0, y: 0 };
let Point { .., y } = p; //~ ERROR expected `}`, found `,`
//~| ERROR pattern does not mention field `x`
//~| ERROR pattern does not mention field `y`
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the pattern mentions field y actually.
Ideally we should do recovery in the parser and continue parsing fields after encountering incorrect .., but it requires somewhat larger refactoring of fn parse_pat_fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

@petrochenkov Would it be possible to just hide the "pattern does not metion field" errors?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Badel2
Yes, but in this case Pat { .., field } or Pat { field, .., } have a single obvious meaning, so it's better to use recovery at syntax level and starting from that assume that Pat { field, .. } was actually implied.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 22, 2018

📌 Commit 347b9d6 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2018
@petrochenkov
Copy link
Contributor

@bors rollup

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 23, 2018
…trochenkov

Better diagnostics for '..' pattern fragment not in the last position

Fixes rust-lang#49257.
@frewsxcv
Copy link
Member

@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 23, 2018
@ordovicia
Copy link
Contributor Author

Seems to conflict with #49160.
I'm going to rebase after it is merged (#49308).

@ordovicia
Copy link
Contributor Author

rebased

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 23, 2018

📌 Commit 3d0ccb2 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 23, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
…trochenkov

Better diagnostics for '..' pattern fragment not in the last position

Fixes rust-lang#49257.
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
…trochenkov

Better diagnostics for '..' pattern fragment not in the last position

Fixes rust-lang#49257.
bors added a commit that referenced this pull request Mar 24, 2018
@bors bors merged commit 3d0ccb2 into rust-lang:master Mar 24, 2018
@ordovicia ordovicia deleted the dotdot-pattern-diag branch March 25, 2018 04:21
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 5, 2018
Accept `..` in incorrect position to avoid further errors

We currently give a specific message when encountering a `..` anywhere
other than the end of a pattern. Modify the parser to accept it (while
still emitting the error) so that we don't also trigger "missing fields
in pattern" errors afterwards.

Add suggestions to either remove trailing `,` or moving the `..` to the
end.

Follow up to rust-lang#49268.
bors added a commit that referenced this pull request Jun 6, 2018
Accept `..` in incorrect position to avoid further errors

We currently give a specific message when encountering a `..` anywhere
other than the end of a pattern. Modify the parser to accept it (while
still emitting the error) so that we don't also trigger "missing fields
in pattern" errors afterwards.

Add suggestions to either remove trailing `,` or moving the `..` to the
end.

Follow up to #49268.
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