(switch) Pattern matching fail on type with implicit cast operator?

@paolomeozzi commented on Fri Dec 29 2017

I have a struct type Gender (a strong enum type):

public struct Gender
{
    public static readonly Gender Male = new Gender(0);
    public static readonly Gender Female = new Gender(1);

    private readonly int value;
    public static Gender Parse(string text)
    {
        text = text.ToUpper();
        if (text == "M")
            return Male;

        if (text == "F")
            return Female;

        throw new FormatException("Invalid format");
    }
    private Gender(int value) => this.value = value;

    public static bool operator ==(Gender left, Gender right) => left.value == right.value;
    public static bool operator !=(Gender left, Gender right) => left.value != right.value;
    public override string ToString() => value == 0 ? "M" : "F";

}

And this code, that work fine:

Gender gender = Gender.Parse("M");
switch (gender)
{
    case Gender g when g == Gender.Male: Console.WriteLine("Male");break;
    case Gender g when g == Gender.Female: Console.WriteLine("Female"); break;
    default:
        break;
}

But, if i add a implicit cast operator:

public struct Gender
{
    ...
    public static implicit operator string(Gender gender) => gender.ToString();
}

The code dot not compile: "Error CS8121 An expression of type 'string' cannot be handled by a pattern of type 'Gender'. "

Same thing if cast operator is int instead of string. But, if i add both cast operators, the switch works well again!

Is a weird behavior or is by design?

---

@svick commented on Fri Dec 29 2017

This seems to be according to the version of the spec that's quoted in the relevant part of the compiler source code:

// SPEC:    The governing type of a switch statement is established by the switch expression.
// SPEC:    1) If the type of the switch expression is sbyte, byte, short, ushort, int, uint,
// SPEC:       long, ulong, bool, char, string, or an enum-type, or if it is the nullable type
// SPEC:       corresponding to one of these types, then that is the governing type of the switch statement. 
// SPEC:    2) Otherwise if exactly one user-defined implicit conversion (§6.4) exists from the
// SPEC:       type of the switch expression to one of the following possible governing types:
// SPEC:       sbyte, byte, short, ushort, int, uint, long, ulong, char, string, or, a nullable type
// SPEC:       corresponding to one of those types, then the result is the switch governing type
// SPEC:    3) Otherwise (in C# 7 and later) the switch governing type is the type of the
// SPEC:       switch expression.

So, if you have exactly one conversion operator to int or string, point 2 applies and your type is converted. If you have zero or more than one such operators, point 3 applies and the type is not converted.

I believe this is because of backwards compatibility: if you had a type with one such conversion operator in C# 6, you could use it in a switch because of the conversion. To make sure C# 7 doesn't break any old code, it still has to apply that conversion.

---

@bondsbw commented on Fri Dec 29 2017

That is an unfortunate rule given that the provided example switch block is not valid for versions less than 7.

It might be too specific a case, but since it is an error in any version then it would not be a breaking change to fix it, a la better betterness.

---

@svick commented on Sat Dec 30 2017

@bondsbw What exactly is your suggestion? That if the switch contains any C# 7-only cases, it should skip point 2? I think that wouldn't be a breaking change, if it was added in C# 7.0, but it's too late for that now. Or do you have a different suggestion?

---

@paolomeozzi commented on Sat Dec 30 2017

@svick Thanks for explanation, but... why would the @bondsbw proposal be a breaking change? The old code (C# 6.0) is not affected by a possible modification of the language; new code currently does not compile; after the change would compile. I do not think it's a breaking change.

I'm not concerned of example posted per sé:

switch (gender)
{
    case Gender g when g == Gender.Male: Console.WriteLine("Male");break;
    case Gender g when g == Gender.Female: Console.WriteLine("Female"); break;
    default:
        break;
}

I think is a rare use of switch. I concerned of this scenario:

1. I realize a type.
2. Others programmers consume my type. (And write code above)
3. I add a function to my type (implicit operator cast) and programmer's code breaks!

That is an unfortunate.

---

@gafter commented on Sat Dec 30 2017

@bondsbw Are you suggesting that the addition of a when clause should cause the switch expression to no longer be subject to the conversion? That is something that people can do (without error) in C# 7, so it would be a breaking change.

---

@paolomeozzi commented on Sat Dec 30 2017

@gafter, I (and @bondsbw, i think) suggest something more specific. Currently, if switch expression is a user defined type (no enum) that defines a (only) implicit cast to primitive type, and exists a case with that type, the code does not compile. This code can not exist in c# 6, so it should be possible to change the language without producing a breaking change.

---

@bondsbw commented on Sat Dec 30 2017

@svick @gafter My suggestion would alter spec item 2) above to something like the following:

2) Otherwise if exactly one user-defined implicit conversion (§6.4) exists from the
   type of the switch expression to one of the following possible governing types:
   sbyte, byte, short, ushort, int, uint, long, ulong, char, string, or, a nullable type
   corresponding to one of those types, 
   *and no case patterns exist which are incompatible with the result,*
   then the result is the switch governing type

So this rule would still apply the conversion if the switch only contains existing valid cases such as these:

case string s when s.StartsWith("M"):
case string s:
case object o:
case string _:

But this rule would not apply if there exists any currently-invalid cases, meaning they would drop to rule 3 and the switch type would become the type Gender (eliminating CS8121):

case Gender g when g == Gender.Male:
case Gender g:

---

@bondsbw commented on Sat Dec 30 2017

Another idea:

Make it so the type isn't determined for the entire switch, but just at each case which would allow a mixture of case Gender g when g == Gender.Male: and a fallback case string s:.

I haven't thought this through so there might be issues with such an interpretation.

---

@HaloFour commented on Sat Dec 30 2017

I think I'd rather wait to see how ADTs play out. I imagine they would just follow some convention as described in this issue. But it would suck if a decision here to make pseudo-ADTs work would box in future design.

---

@gafter commented on Sun Dec 31 2017

The LDM did consider these options. Early on we favored some of the suggested approaches, but found them problematic:

• If we make some cases use the original value and some use the converted value, it is hard to reason about the set of cases as a whole, and some of them may have unexpected meaning due to the conversion being unexpectedly applied or not applied.
• If we make the conversion depend on whether there are any "new" constructs, then the addition of a "where" clause near the bottom of the switch can affect the meaning of the expression at the top of the switch. That kind of nonlocal behavior is usually confusing.
• A similar issue occurs if we depend on pattern compatibility.

If you are so unfortunate that the author of the type you're switching on provided a conversion to a switchable type, then you can cast your switch expression to object to avoid the conversion occurring. The LDM felt that this was an easy enough workaround for the problem that no more complicated accommodation would be needed.

---

@bondsbw commented on Sun Dec 31 2017

Makes sense. In that case, I suggest extending the error message to explain that the implicit cast is being applied and perhaps hint at the object cast suggestion by @gafter.

---

@gafter commented on Wed Jan 02 2019

Moving to Roslyn for a possibly improved diagnostic.