Roslyn: Roslyn produces wrong code for huge switch expression

[sys27]

Version Used:
.NET Core SDK 3.1.302
.NET 5 RC1

Steps to Reproduce:

Use the following source code:

using System;
public class C {
    public static int M0(int x)
    {
        var b1 = false;
        var b2 = false;
        var b3 = false;
        var b4 = false;
        var b5 = false;
        var b6 = false;
        var b7 = true; // but, if you remove these two variables b7, b8
        var b8 = true; // then everything is ok

        return (x, b1, b2, b3, b4, b5, b6, b7, b8) switch
        {
            (1, false, false, false, false, false, false, true, true) => 1,
            _ => -1,
        };
    }

    public static void Main() {
        Console.WriteLine(M0(1));
    }
}

It will produce:

using System;
public class C
{
    public static int M0(int x)
    {
        bool flag = false;
        bool flag2 = false;
        bool flag3 = false;
        bool flag4 = false;
        bool flag5 = false;
        bool flag6 = false;
        bool flag7 = true;
        bool flag8 = true;
        bool flag9 = default(bool);   // these vars are not present in source code
        bool flag10 = default(bool);  // they are used instead of the "real" ones
        if (x == 1 && !flag && !flag2 && !flag3 && !flag4 && !flag5 && !flag6 && flag9 && flag10)
        {
            return 1;
        }
        return -1;
    }

    public static void Main()
    {
        Console.WriteLine(M0(1));
    }
}

Expected Behavior:
Provided sample should print "1".

Actual Behavior:
Provided sample will print "-1" (wrong code is generated).

[Youssef1313]

Simpler repro:

https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxAgrgOyQExAagB8ABAJgEYBYAKGIGYACMhgYQYG8aHunHjyAbAwCWmGAwCyABgAUASi492PRTx7EA7AxnkkDXfr0HjRo3IZQA7sJgBjABaq1HJ8547Thrye/mAvAB8hq5uDAD6DIEMALS6IdwAvgDcTglOTmSkLtShTOQAnDLS8nIpOTxp1AlAA===

[Fabi]

Declaring (1, 1, 1, 1, 1, 1, 1, 1) in a new variable and switching into it actually works 🤔

[Youssef1313]

In case this is useful, this bug is a regression happened somewhere between October 2019 and November 2019.

Branch demos/records on SharpLab is working as expected. (The last commit for it was merging master to it in 7 Oct 2019.

Commit 70e158ba6c2c99bd3c3fc0754af0dbf82a6d353d (05 Nov 2019) (just chosen randomly :smile:), is tested locally and has the buggy behavior.

[gafter]

It appears the pattern lowering code doesn't handle long tuples members correctly.
/cc @jaredpar

[Youssef1313]

Testing with tuple of 7 values (which behaves correctly):

            var text = @"
using System;
public class C
{   
    public static void Main()
    {
        int x = (1, 1, 1, 1, 1, 1, 1) switch
        {
            (1, 1, 1, 1, 1, 1, 1) => 1,
            _ => -1,
        };
        Console.WriteLine(x);
    }
}
";
            var comp = CompileAndVerify(text, expectedOutput: "1");
            comp.VerifyDiagnostics();

visited.Arguments has length 7, and all of them are BoundLiteral 1.

---

Testing with tuple of 8 values (which behaves incorrectly):

            var text = @"
using System;
public class C
{   
    public static void Main()
    {
        int x = (1, 1, 1, 1, 1, 1, 1, 1) switch
        {
            (1, 1, 1, 1, 1, 1, 1, 1) => 1,
            _ => -1,
        };
        Console.WriteLine(x);
    }
}
";
            var comp = CompileAndVerify(text, expectedOutput: "1");
            comp.VerifyDiagnostics();

visited.Arguments has length 8, but the last item is not BoundLiteral 1, while I think it should have been BoundLiteral 1. (Edit: Maybe not?)

Posting this just in case it can save time if someone is trying to look.

Note: The reason why this bug occurs when the tuple contains 8 or more elements is most likely due to ValueTuple being able to hold at maximum 7 elements. For larger than that, the runtime provides ValueTuple<T1, T2, T3, T4, T5, T6, T7, TRest> where TRest is a tuple itself.

Note 2: The issue might be in MakeTupleCreationExpression method. (Edit: was mistaken.)

Note 3: The PR dotnet/roslyn#39069 is around the timing of this regression. It's worth having a look to it.

Note 4: The old correctly working branch was using unsafe code:

SharpLab

Input:

    public void M() {
                int x = (1, 1, 1, 1, 1, 1, 1, 1) switch
        {
            (1, 1, 1, 1, 1, 1, 1, 1) => 1,
            _ => -1,
        };
        Console.WriteLine(x);
    }

is compiled to:

    public unsafe void M()
    {
        int num = default(int);
        *(ValueTuple<int>*)(&num) = new ValueTuple<int>(1);
        int num2 = 1;
        int num3 = 1;
        int num4 = 1;
        int num5 = 1;
        int num6 = 1;
        int num7 = 1;
        int num8 = 1;
        int num9 = (num8 == 1 && num7 == 1 && num6 == 1 && num5 == 1 && num4 == 1 && num3 == 1 && num2 == 1 && num == 1) ? 1 : (-1);
        int value = num9;
        Console.WriteLine(value);
    }

---

Note 5: This is a half regression 😄

Currently, the code is lowered incorrectly whenever the tuple has >= 8 elements.

In the past:

It worked correctly for 8 elements (the generated code was using pointers in unsafe context).
But for > 8 elements, the compiler was crashing.

[Youssef1313]

In PatternLocalRewriter:

            private BoundDecisionDag RewriteTupleInput(
                BoundDecisionDag decisionDag,
                BoundObjectCreationExpression loweredInput,
                Action<BoundExpression> addCode,
                bool canShareInputs,
                out BoundExpression savedInputExpression)
            {
                int count = loweredInput.Arguments.Length;

                // first evaluate the inputs (in order) into temps
                var originalInput = BoundDagTemp.ForOriginalInput(loweredInput.Syntax, loweredInput.Type);
                var newArguments = ArrayBuilder<BoundExpression>.GetInstance(loweredInput.Arguments.Length);
                for (int i = 0; i < count; i++)
                {
                    var field = loweredInput.Type.TupleElements[i].CorrespondingTupleField;
                    Debug.Assert(field != null);
                    var expr = loweredInput.Arguments[i];
                    var fieldFetchEvaluation = new BoundDagFieldEvaluation(expr.Syntax, field, originalInput);
                    var temp = new BoundDagTemp(expr.Syntax, expr.Type, fieldFetchEvaluation);
                    storeToTemp(temp, expr);
                    newArguments.Add(_tempAllocator.GetTemp(temp));
                }

The problematic line seems to be : var expr = loweredInput.Arguments[i];

When i = 8, expr will be Tuple<int> instead of int because it's represented as ValueTuple<int, int, int, int, int, int, int, ValueTuple<int>

[RikkiGibson]

Fixed by #48028. Thank you for reporting.