Roslyn: Suboptimal code for type pattern matching in a switch statement

[SergeyTeplyakov]

The compiler is smart enough to group different clauses with the same type together:

public void SwitchBasedPatternMatching2(object o)
{
    switch (o)
    {
        case int n when n == 1:
            Console.WriteLine("1"); break;
        case int n when n == 2:
            Console.WriteLine("2"); break;
        case string s:
            Console.WriteLine("s"); break;
    }
}

The generated code roughly looks like this:

public void SwitchBasedPatternMatching2(object o)
{
    if ( o != null)
    {
        bool isInt = o is int;
        int num = isInt ? ((int)o) : 0;
        if (isInt)
        {
            if (num == 1)
            {
                Console.WriteLine("1");
                return;
            }
            if (num == 2)
            {
                Console.WriteLine("2");
                return;
            }
        }
        string text;
        if ((text = (o as string)) != null)
        {
            Console.WriteLine("s");
        }
    }
}

If the switch statement has more than one consecutive type patterns with the same type the compiler will check the type only once and will check different predicates inside the if statement. In the case above it means that only one unboxing operation will happen.

But if the cases clauses intermixed with each other, then the code is not that optimal:

public void SwitchBasedPatternMatching(object o)
{
    switch (o)
    {
        case int n when n == 1:
            Console.WriteLine("1"); break;
        case string s:
            Console.WriteLine("s"); break;
        case int n when n == 2:
            Console.WriteLine("2"); break;
    }
}

In this case, the generated code checks that o is int two times:

```csharp
public void SwitchBasedPatternMatching(object o)
{
if (o != null)
{
bool isInt1 = o is int;
int num = isInt1 ? ((int)o) : 0;
if (isInt1 && num == 1)
{
Console.WriteLine("1");
return;
}
string text;
if ((text = (o as string)) != null)
{
Console.WriteLine("s");
return;
}
bool isInt2 = o is int;
num = (isInt2 ? ((int)o) : 0);
if (isInt2 && num == 2)
{
Console.WriteLine("2");
}
}
}
````

It is clear that the order of the case clauses matters know. But I can't see why the compiler can't group different cases together based on a type and generate effectively the same code as for the case before.

Is there a use case when this optimization will have an observable side effect or there is a room for optimization here?

[alrz]

Another thing:

        bool isInt = o is int;
        int num = isInt ? ((int)o) : 0;
        if (isInt && whenCondition)

The cast (int)o should be postponed until after whenCondition is evaluated.

[SergeyTeplyakov]

In this case, if there is more than one type pattern with the same type the generated code will unbox the value more than once.

[jcouv]

Tagging @gafter

[gafter]

The pattern-matching lowering code is being rewritten from scratch (to support recursive patterns, too). I expect most of the improvements you seek here will come for "free" in the new code. But it will be some time before that rewrite is ready for prime time.

The cast (int)o should be postponed until after whenCondition is evaluated.

No, the whenCondition can (and typically does) reference the pattern variable.

[gafter]

I am adding the following test to the verify that this will be fixed in C# 8.0.

c# [Fact, WorkItem(22654, "https://github.com/dotnet/roslyn/issues/22654")] public void NoRedundantTypeCheck() { var source = @"using System; public class C { public void SwitchBasedPatternMatching(object o) { switch (o) { case int n when n == 1: Console.WriteLine(""1""); break; case string s: Console.WriteLine(""s""); break; case int n when n == 2: Console.WriteLine(""2""); break; } } }"; var compilation = CreateCompilation(source, options: TestOptions.ReleaseDll); compilation.VerifyDiagnostics(); var compVerifier = CompileAndVerify(compilation); compVerifier.VerifyIL("C.SwitchBasedPatternMatching", @"{ // Code size 77 (0x4d) .maxstack 2 .locals init (object V_0, int V_1, string V_2) IL_0000: ldarg.1 IL_0001: stloc.0 IL_0002: ldloc.0 IL_0003: isinst ""int"" IL_0008: brfalse.s IL_0013 IL_000a: ldloc.0 IL_000b: unbox.any ""int"" IL_0010: stloc.1 IL_0011: br.s IL_0024 IL_0013: ldloc.0 IL_0014: isinst ""string"" IL_0019: brfalse.s IL_004c IL_001b: ldloc.0 IL_001c: castclass ""string"" IL_0021: stloc.2 IL_0022: br.s IL_0033 IL_0024: ldloc.1 IL_0025: ldc.i4.1 IL_0026: bne.un.s IL_003e IL_0028: ldstr ""1"" IL_002d: call ""void System.Console.WriteLine(string)"" IL_0032: ret IL_0033: ldstr ""s"" IL_0038: call ""void System.Console.WriteLine(string)"" IL_003d: ret IL_003e: ldloc.1 IL_003f: ldc.i4.2 IL_0040: bne.un.s IL_004c IL_0042: ldstr ""2"" IL_0047: call ""void System.Console.WriteLine(string)"" IL_004c: ret }"); }

[gafter]

Test added to confirm fixed in https://github.com/dotnet/roslyn/issues/25567