Skip to content

Give an example in the non-exhaustive diagnostic#44702

Merged
gafter merged 12 commits intodotnet:masterfrom
gafter:patterns3-nonexhaustive-example
Jun 23, 2020
Merged

Give an example in the non-exhaustive diagnostic#44702
gafter merged 12 commits intodotnet:masterfrom
gafter:patterns3-nonexhaustive-example

Conversation

@gafter
Copy link
Member

@gafter gafter commented May 29, 2020

Fixes #43943

@gafter gafter added this to the Compiler.Net5 milestone May 29, 2020
@gafter gafter self-assigned this May 29, 2020
@gafter gafter added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label May 29, 2020
alrz
alrz reviewed Jun 3, 2020
View reviewed changes
@gafter gafter force-pushed the patterns3-nonexhaustive-example branch from bcfdde5 to 8dd7aed Compare June 10, 2020 02:43
@gafter
Copy link
Member Author

gafter commented Jun 10, 2020 •
edited
Loading

@alrz Are there any tests you suggest I add before opening this up for review? #Resolved

@alrz
Copy link
Member

alrz commented Jun 10, 2020 •
edited by gafter
Loading

I wonder what example we give for:

return unsigned switch 
{
  0 => true,
  var x when x > 0 => false,
};

I would expect _ since any other number is caught by the second arm. F# gives the example anyways but has a special diagnostic in presence of when. For example, the value '1' may indicate a case not covered by the pattern(s). However, a pattern rule with a 'when' clause might successfully match this value. #Resolved

@gafter
Copy link
Member Author

gafter commented Jun 10, 2020

We give the example '1', as we don't know if the when clause will succeed. However, we can add that additional note to the diagnostics when a when clause is involved. Let me work on that...

In reply to: 641706686 [](ancestors = 641706686)

@gafter
Copy link
Member Author

gafter commented Jun 14, 2020 •
edited
Loading

Actually, I'd like to open this up for review and handle the "However" clause of the diagnostic in a separate PR (#45157). But I definitely agree we should do that. #Resolved

gafter
gafter commented Jun 14, 2020
View reviewed changes
@gafter gafter marked this pull request as ready for review June 14, 2020 02:11
@gafter gafter requested a review from a team as a code owner June 14, 2020 02:11
@gafter gafter removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jun 14, 2020
@gafter
Copy link
Member Author

gafter commented Jun 14, 2020 •
edited
Loading

@dotnet/roslyn-compiler May I please have a couple of reviews? #Resolved

This was referenced Jun 14, 2020
Closed
Closed
@gafter gafter requested a review from a team as a code owner June 16, 2020 19:03
RikkiGibson
RikkiGibson reviewed Jun 16, 2020
View reviewed changes
RikkiGibson
RikkiGibson reviewed Jun 17, 2020
View reviewed changes
Minor changes per code review.
2b1d20f 
- Fix a typo
- Make IValueSet methods throw an exception when empty
- Simplify NumericValueSet's selection when there are no positive values
alrz
alrz reviewed Jun 17, 2020
View reviewed changes
Handle extension methods vs instance methods.
6c9518b 
Properly produce `{ }` after all single-element deconstructions.
@gafter
Copy link
Member Author

gafter commented Jun 18, 2020 •
edited
Loading

@dotnet/roslyn-compiler May I please have a review from a Sr engineer? #Resolved

cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 19, 2020
View reviewed changes
cston
cston reviewed Jun 20, 2020
View reviewed changes
@cston
Copy link
Contributor

cston commented Jun 20, 2020 •
edited
Loading 

        private ImmutableHashSet<T> _membersIncludedOrExcluded;

Fields can be readonly. #Closed

Refers to: src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.EnumeratedValueSet.cs:30 in 7063b73. [](commit_id = 7063b73, deletion_comment = False)

cston
cston reviewed Jun 22, 2020
View reviewed changes
cston
cston reviewed Jun 22, 2020
View reviewed changes
src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs
var elementTemp = new BoundDagTemp(e.Syntax, e.Field.Type, e);
var index = e.Field.TupleElementIndex;
if (index < 0 || index >= cardinality)
return null;
Copy link
Contributor

@cston cston Jun 22, 2020 •
edited by gafter
Loading

Choose a reason for hiding this comment

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

return null [](start = 28, length = 11)

Are we testing this case? #Resolved

Copy link
Member Author

@gafter gafter Jun 22, 2020

Choose a reason for hiding this comment

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

No. It would just cause us not to produce a tuple pattern in the face of a bad tuple API declaration. We would use a property pattern instead with Item1 etc.

In reply to: 443843360 [](ancestors = 443843360)

Copy link
Member Author

@gafter gafter Jun 22, 2020

Choose a reason for hiding this comment

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

Added a test,

In reply to: 443845046 [](ancestors = 443845046,443843360)

jasonmalinowski
jasonmalinowski approved these changes Jun 23, 2020
View reviewed changes
Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Signing off on the skipping of the IDE test only.

@gafter gafter merged commit 212fc8f into dotnet:master Jun 23, 2020
@ghost ghost modified the milestones: Compiler.Net5, Next Jun 23, 2020
@dibarbet dibarbet modified the milestones: Next, 16.7.P4 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonmalinowski jasonmalinowski jasonmalinowski approved these changes

@RikkiGibson RikkiGibson RikkiGibson approved these changes

+2 more reviewers

@alrz alrz alrz left review comments

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@gafter gafter

Labels

Area-Compilers Feature - Pattern Matching Pattern Matching

Projects

None yet

Milestone

16.7.P4

Development

Successfully merging this pull request may close these issues.

Give an example of unmatched patterns in non-exhaustive switch warnings

6 participants

@gafter @alrz @cston @jasonmalinowski @RikkiGibson @dibarbet