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

Pattern matching: completeness vs bool and enum types #11438

Closed
gafter opened this issue May 19, 2016 · 10 comments
Closed

Pattern matching: completeness vs bool and enum types #11438

gafter opened this issue May 19, 2016 · 10 comments
Assignees
Labels
Area-Compilers Area-Language Design Feature Request New Language Feature - Pattern Matching Pattern Matching Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@gafter
Copy link
Member

gafter commented May 19, 2016

If a switch statement handles cases for true and false, should there be a warning that a default case's body isn't reachable?

bool b = ...;
switch (b)
{
    case true:
    case false:
        break;
    default:
        break; // warning: unreachable code?
}

Similar questions for (non-flags) enum types (when there are cases for all the named members).

For backward compatibility, we probably cannot add such new warnings. Also, it is actually possible to get values outside these named examples (e.g., one can construct a bool that contains the value 23).

Perhaps such warnings are deserved in a new expression-based match?

@paulomorgado
Copy link

They should, definitely, not be errors!

And I can always #pragma those warnings out, right?

@gafter
Copy link
Member Author

gafter commented May 20, 2016

@paulomorgado Sure, but the more interesting question is what happens if we require match expressions to be complete? If you handle both true and `false, do we require a catch-all case, or do we forbid it?

@HaloFour
Copy link

If a switch statement handles cases for true and false, should there be a warning that a default case's body isn't reachable?

I think so, but only as a part of #1580.

one can construct a bool that contains the value 23

Which is still true.

Similar questions for (non-flags) enum types (when there are cases for all the named members).

This one is tougher. Given that nothing in C# nor the runtime can prevent someone from explicitly casting some garbage value to an enum I want to say that you can't technically consider the match complete without a default/wildcard case. But it should be an exceptional case. Personally I think that there should be no warning for an extraneous default case, but it also shouldn't be required in considering completeness.

@svick
Copy link
Contributor

svick commented May 20, 2016

Right now, this is en error ("CS0161 'IsA(Foo)': not all code paths return a value"):

enum Foo
{
    A, B
}

bool IsA(Foo foo)
{
    switch(foo)
    {
    case Foo.A:
        return true;
    case Foo.B:
        return false;
    }
}

I think a sensible way to make that code compile is to add default:

bool IsA(Foo foo)
{
    switch(foo)
    {
    case Foo.A:
        return true;
    case Foo.B:
        return false;
    default:
        throw new Exception();
    }
}

But now that would be a warning? Unless the first sample above worked in C# 7, I think adding the warning for enums is a bad idea. And probably even then.

I would like to have "strong" enums, which can't have values that are not named. But the reality is that enums can have such values, and C# directly supports them. So the default is actually reachable and the warning would be incorrect.

@alrz
Copy link
Contributor

alrz commented May 20, 2016

@svick I think enum struct in (#6739) would be a good candidate for complete enums, I don't mind being able to call the constructor in each member, so it wouldn't look like Java enums.

@Unknown6656
Copy link

I personally think, that a warning should be issued for the default-case, and (if the CLR allows this) the possibility to omit the default-case, as there is no sense in using it...

@vladd
Copy link

vladd commented May 22, 2016

@gafter

Also, it is actually possible to get values outside these named examples (e.g., one can construct a bool that contains the value 23).

Strange enough, the C# 5.0 Language Spec (4.1.8) states that "The possible values of type bool are true and false." Doesn't your statement contradict the specs? Or is this not true any more?

@paulomorgado
Copy link

@gafter, as @HaloFour mentioned, in practice, Booleans are only true or false. Having a default won't change the meaning of the program. But there's something wrong if the developer adds a default clause. So, that should be a warning.

As for enums (flags or not) they can assume any value of its base type. So, a match expression needs a default clause.

@gafter
Copy link
Member Author

gafter commented Jul 14, 2016

We resolved to handle this as follows:

  • For bool, handling both true and false means all input values are handled.
  • We never warn about a default case being unreachable or unnecessary.
  • We don't handle completeness for enum types, as there are a huge number of values even if only a few are named.

This is currently implemented in master.

@gafter
Copy link
Member Author

gafter commented Feb 15, 2018

In the future, I will be using the term exhaustive/exhaustiveness instead of complete/completeness for this aspect of pattern-matching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Area-Language Design Feature Request New Language Feature - Pattern Matching Pattern Matching Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

No branches or pull requests

7 participants