Rename `overlapping_patterns` lint #78242

Nadrieril · 2020-10-22T18:36:09Z

As discussed in #65477. I also tweaked a few things along the way.

r? @varkor
@rustbot modify labels: +A-exhaustiveness-checking

varkor · 2020-10-22T20:06:52Z

@rust-lang/lang: this renames overlapping_patterns to overlapping_range_endpoints to address the confusing description of the lint, as discussed in #65477. This probably requires an FCP, or a sign-off.

The implementation itself looks fine.

nikomatsakis · 2020-10-22T20:37:18Z

Nominating to discuss the name in the lang-team meeting.

bors · 2020-10-25T00:56:06Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

bors · 2020-10-29T04:24:55Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

bors · 2020-11-01T19:41:16Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Nadrieril · 2020-11-18T19:58:15Z

From watching the triage meeting it seems the reason for renaming the lint wasn't super clear, so I thought I'd clarify.

The issue is that overlapping_patterns sounds like it should lint on any kind of patterns that overlap. But that's really not what it does. What it does is specifically check if two ranges have a common endpoint, which could likely be a mistake:

match x {
	0..=10 => {} // Oops, probably meant to write `0..10`
	10..=20 => {}
	_ => {}
}

Knowing only the name of the lint, one would expect it to lint may more things. Even knowing that it applies to ranges, one would expect it to lint for overlaps like the following, but it doesn't:

match x {
	0..10 => {}
	5..15 => {}
	_ => {}
}

So it was proposed to rename it to overlapping_range_endpoints so that the name is an accurate description of what it lints for.

scottmcm · 2020-11-19T07:08:14Z

Thanks for watching and commenting! I'd certainly forgotten what this was about.

sounds like it should lint on any kind of patterns that overlap. But that's really not what it does.

I think that's a great summary, and I agree with niko's statement that a better name that other people like is good by me. So

@rfcbot merge

rfcbot · 2020-11-19T07:08:15Z

Team member @scottmcm has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

nikomatsakis · 2020-11-19T13:47:49Z

Yes, thanks for clarifying! And glad to know people watch the recordings. =)

@rfcbot reviewed

bors · 2020-11-22T21:17:51Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

bors · 2020-11-28T01:07:46Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

bors · 2020-11-29T21:18:57Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

rfcbot · 2020-12-08T18:24:32Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2020-12-18T18:25:50Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

varkor

Implementation looks good. Just a couple of suggestions about the diagnostics changes.

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

varkor · 2020-12-19T22:53:10Z

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

@@ -320,7 +318,8 @@ impl IntRange {
                            ),
                        );
                    }
-                    err.note("this is likely to be a mistake");
+                    err.span_label(pcx.span, "... with this range");


Ah, this didn't have the effect I hoped it would. I forget how vertical label ordering is determined: I guess by span location?

I'm not sure, but I think it's only the tests that involve macros that are confusing. In normal cases the ranges will be on different lines of a match and the messages will be in the order we want them

Yes, that's true. Not worth worrying about.

varkor · 2020-12-21T19:39:36Z

@bors r+

Thanks!

bors · 2020-12-21T19:39:38Z

📌 Commit 5b6c175 has been approved by varkor

…nts-lint, r=varkor Rename `overlapping_patterns` lint As discussed in rust-lang#65477. I also tweaked a few things along the way. r? `@varkor` `@rustbot` modify labels: +A-exhaustiveness-checking

…nts-lint, r=varkor Rename `overlapping_patterns` lint As discussed in rust-lang#65477. I also tweaked a few things along the way. r? ``@varkor`` ``@rustbot`` modify labels: +A-exhaustiveness-checking

…nts-lint, r=varkor Rename `overlapping_patterns` lint As discussed in rust-lang#65477. I also tweaked a few things along the way. r? ```@varkor``` ```@rustbot``` modify labels: +A-exhaustiveness-checking

bors · 2020-12-22T10:32:08Z

⌛ Testing commit 5b6c175 with merge 75e1acb...

bors · 2020-12-22T13:19:33Z

☀️ Test successful - checks-actions
Approved by: varkor
Pushing 75e1acb to master...

…Simulacrum lint-docs: Warn on missing lint when documenting. In rust-lang#79522, I missed converting one of the errors to a warning, in the situation where a lint is missing. This was unearthed by the renaming of overlapping-patterns (rust-lang#78242). This will still be validated during the test phase.

rustbot added the A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns label Oct 22, 2020

rust-highfive assigned varkor Oct 22, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 22, 2020

nikomatsakis added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Oct 22, 2020

varkor added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2020

Nadrieril force-pushed the rename-overlapping_endpoints-lint branch 2 times, most recently from cccf902 to a856dd2 Compare October 25, 2020 15:59

Nadrieril force-pushed the rename-overlapping_endpoints-lint branch from a856dd2 to 1ee30f1 Compare October 30, 2020 01:30

Nadrieril force-pushed the rename-overlapping_endpoints-lint branch from 1ee30f1 to f453432 Compare November 1, 2020 20:19

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 19, 2020

Nadrieril force-pushed the rename-overlapping_endpoints-lint branch from f453432 to c62df4b Compare November 22, 2020 22:27

Nadrieril force-pushed the rename-overlapping_endpoints-lint branch from c62df4b to 5afe60f Compare November 28, 2020 21:04

Nadrieril added 3 commits November 29, 2020 21:29

Add tests

06ca6bb

Rename the overlapping_patterns lint to overlapping_range_endpoints

d1a50ff

Be consistent about linting singletons

c89d439

Mark-Simulacrum removed the I-nominated label Dec 8, 2020

rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Dec 18, 2020

rfcbot added the to-announce Announce this issue on triage meeting label Dec 18, 2020

varkor reviewed Dec 19, 2020

View reviewed changes

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs Outdated Show resolved Hide resolved

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs Outdated Show resolved Hide resolved

Tweak diagnostics

5b6c175

varkor reviewed Dec 19, 2020

View reviewed changes

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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Dec 21, 2020

Dylan-DPC-zz mentioned this pull request Dec 21, 2020

Rollup of 15 pull requests #80276

Closed

Dylan-DPC-zz mentioned this pull request Dec 22, 2020

Rollup of 10 pull requests #80281

Closed

Dylan-DPC-zz mentioned this pull request Dec 22, 2020

Rollup of 9 pull requests #80282

Closed

Nadrieril mentioned this pull request Dec 22, 2020

Clarify constructor splitting in exhaustiveness checking #80242

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 22, 2020

bors merged commit 75e1acb into rust-lang:master Dec 22, 2020

rustbot added this to the 1.50.0 milestone Dec 22, 2020

Nadrieril deleted the rename-overlapping_endpoints-lint branch December 22, 2020 15:19

ehuss mentioned this pull request Dec 26, 2020

lint-docs: Warn on missing lint when documenting. #80395

Merged

spastorino removed the to-announce Announce this issue on triage meeting label Dec 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename `overlapping_patterns` lint #78242

Rename `overlapping_patterns` lint #78242

Nadrieril commented Oct 22, 2020

varkor commented Oct 22, 2020

nikomatsakis commented Oct 22, 2020

bors commented Oct 25, 2020

bors commented Oct 29, 2020

bors commented Nov 1, 2020

Nadrieril commented Nov 18, 2020

scottmcm commented Nov 19, 2020

rfcbot commented Nov 19, 2020 •

edited by cramertj

Loading

nikomatsakis commented Nov 19, 2020

bors commented Nov 22, 2020

bors commented Nov 28, 2020

bors commented Nov 29, 2020

rfcbot commented Dec 8, 2020

rfcbot commented Dec 18, 2020

varkor left a comment

varkor Dec 19, 2020

Nadrieril Dec 20, 2020 •

edited

Loading

varkor Dec 21, 2020

varkor commented Dec 21, 2020

bors commented Dec 21, 2020

bors commented Dec 22, 2020

bors commented Dec 22, 2020

Rename overlapping_patterns lint #78242

Rename overlapping_patterns lint #78242

Conversation

Nadrieril commented Oct 22, 2020

varkor commented Oct 22, 2020

nikomatsakis commented Oct 22, 2020

bors commented Oct 25, 2020

bors commented Oct 29, 2020

bors commented Nov 1, 2020

Nadrieril commented Nov 18, 2020

scottmcm commented Nov 19, 2020

rfcbot commented Nov 19, 2020 • edited by cramertj Loading

nikomatsakis commented Nov 19, 2020

bors commented Nov 22, 2020

bors commented Nov 28, 2020

bors commented Nov 29, 2020

rfcbot commented Dec 8, 2020

rfcbot commented Dec 18, 2020

varkor left a comment

Choose a reason for hiding this comment

varkor Dec 19, 2020

Choose a reason for hiding this comment

Nadrieril Dec 20, 2020 • edited Loading

Choose a reason for hiding this comment

varkor Dec 21, 2020

Choose a reason for hiding this comment

varkor commented Dec 21, 2020

bors commented Dec 21, 2020

bors commented Dec 22, 2020

bors commented Dec 22, 2020

Rename `overlapping_patterns` lint #78242

Rename `overlapping_patterns` lint #78242

rfcbot commented Nov 19, 2020 •

edited by cramertj

Loading

Nadrieril Dec 20, 2020 •

edited

Loading