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.

Sign up for GitHub

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

Jump to bottom

Reduce the diagnostic spam when multiple fields are missing in pattern #49160

Merged
merged 3 commits into from
Mar 23, 2018
Merged

Reduce the diagnostic spam when multiple fields are missing in pattern #49160

merged 3 commits into from
Mar 23, 2018

Conversation

estebank
Copy link
Contributor

Fix #47457.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 19, 2018
@estebank estebank changed the title Reduce the diagnostic span when multiple fields are missing in pattern Reduce the diagnostic spam when multiple fields are missing in pattern Mar 19, 2018
oli-obk
oli-obk requested changes Mar 19, 2018
View reviewed changes
Copy link
Contributor

@oli-obk oli-obk left a comment •
edited
Loading

Choose a reason for hiding this comment

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

r? @oli-obk iirc eddyb doesn't like diagnostics PRs

src/test/ui/missing-fields-in-struct-pattern.rs
// option. This file may not be copied, modified, or distributed
// except according to those terms.

struct S(usize, usize, usize, usize);
Copy link
Contributor

Choose a reason for hiding this comment

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

did you forget to generate or git add a .stderr file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

src/librustc_typeck/check/_match.rs Outdated
field_names);
for (span, name) in &inexistent_fields {
err.span_label(*span,
format!("{} `{}` does not have field `{}`",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the labels could just be unknown field instead of repeating all information

Copy link
Contributor

Choose a reason for hiding this comment

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

or, since these are usually very compact, do something similar to use imports:

warning: unused imports: `u32`, `usize`
 --> src/main.rs:1:11
  |
1 | use std::{usize, u32};
  |           ^^^^^  ^^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@oli-obk oli-obk assigned oli-obk and unassigned eddyb Mar 19, 2018
@estebank estebank force-pushed the issue-47457-missing-fields branch 3 times, most recently from 3fff16c to 97cf809 Compare March 19, 2018 19:53
@estebank estebank force-pushed the issue-47457-missing-fields branch 5 times, most recently from 16987ae to 0d6ba1d Compare March 20, 2018 01:17
@oli-obk
Copy link
Contributor

oli-obk commented Mar 20, 2018

@bors r+

Looks sooo much better than the old spammy messages!

@bors
Copy link
Contributor

bors commented Mar 20, 2018

📌 Commit 062a46f has been approved by oli-obk

@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 20, 2018
@estebank
Copy link
Contributor Author

@bors rollup

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 23, 2018
@frewsxcv
Rollup merge of rust-lang#49160 - estebank:issue-47457-missing-fields…
c783019 
…, r=oli-obk

Reduce the diagnostic spam when multiple fields are missing in pattern

Fix rust-lang#47457.
@frewsxcv frewsxcv mentioned this pull request Mar 23, 2018
Closed
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 23, 2018
@frewsxcv
Rollup merge of rust-lang#49160 - estebank:issue-47457-missing-fields…
f3fdc53 
…, r=oli-obk

Reduce the diagnostic spam when multiple fields are missing in pattern

Fix rust-lang#47457.
@frewsxcv frewsxcv mentioned this pull request Mar 23, 2018
Closed
@alexcrichton alexcrichton mentioned this pull request Mar 23, 2018
Merged
@ordovicia ordovicia mentioned this pull request Mar 23, 2018
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 23, 2018
@alexcrichton
Rollup merge of rust-lang#49160 - estebank:issue-47457-missing-fields…
401a930 
…, r=oli-obk

Reduce the diagnostic spam when multiple fields are missing in pattern

Fix rust-lang#47457.
bors added a commit that referenced this pull request Mar 23, 2018
@bors
Auto merge of #49308 - alexcrichton:rollup, r=alexcrichton
c08480f 
Rollup of 15 pull requests

- Successful merges: #48265, #48528, #48552, #48624, #48883, #48909, #49028, #49030, #49102, #49160, #49169, #49203, #49262, #49272, #49295
- Failed merges: #48942, #49035
@bors
Copy link
Contributor

bors commented Mar 23, 2018

☔ The latest upstream changes (presumably #49308) made this pull request unmergeable. Please resolve the merge conflicts.

@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
@alexcrichton alexcrichton merged commit 062a46f into rust-lang:master Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@estebank @rust-highfive @oli-obk @bors @alexcrichton @eddyb