Switch statement broken in Roslyn

[mdaveynis]

Following test-case fails only if using Roslyn (it works on a smaller amount of case statements, i.e. if case "G" is removed):

//@useroslyn

using System;

public static class Program {

    public static void Main (string[] args)
    {
        var a = GetValue();
        switch (a)
        {
            case "A":
                Console.Write("A");
                break;

            case "B":
                Console.Write("B");
                break;
            case "C":
                Console.Write("C");
                break;
            case "D":
                Console.Write("D");
                break;

            case "E":
                Console.Write("E");
                break;

            case "F":
                Console.Write("F");
                break;

           case "G":
                Console.Write("G");
                break;
        }
    }

    private static string GetValue()
    {
        return "C";
    }
}

Translation result:

  function Program_GetValue () {
    return "C";
  }; 

  function Program_Main (args) {
    var a = $thisType.GetValue();
    var num = (JSIL.IgnoredMember("ComputeStringHash(s)", a).LValue >>> 0);
    if (num <= 3255563174) {
      if (num !== 3222007936) {
        if (num !== 3238785555) {
          if (num !== 3255563174) {
            return;
          }
          if (!(a == "G")) {
            return;
          }
          $T03().Write("G");
          return;
        } else {
          if (!(a == "D")) {
            return;
          }
          $T03().Write("D");
          return;
        }
      } else {
        if (!(a == "E")) {
          return;
        }
        $T03().Write("E");
        return;
      }
    } else if (num <= 3289118412) {
      if (num !== 3272340793) {
        if (num !== 3289118412) {
          return;
        }
        if (!(a == "A")) {
          return;
        }
        $T03().Write("A");
        return;
      } else {
        if (!(a == "F")) {
          return;
        }
        $T03().Write("F");
        return;
      }
    } else if (num !== 3322673650) {
      if (num !== 3339451269) {
        return;
      }
      if (!(a == "B")) {
        return;
      }
      $T03().Write("B");
      return;
    } else {
      if (!(a == "C")) {
        return;
      }
      $T03().Write("C");
      return;
    }
  }; 

[kg]

Whoa, that is SUPER interesting codegen... and also relies on the specific implementation of string hashing >_<

[iskiselev]

Really they add hashing finction inside compiled assembly. Here it is:

.class private auto ansi sealed '<PrivateImplementationDetails>'
       extends [mscorlib]System.Object
{
  .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) 
} // end of class '<PrivateImplementationDetails>'

.method assembly hidebysig static uint32 
        ComputeStringHash(string s) cil managed
{
  // Code size       46 (0x2e)
  .maxstack  2
  .locals init (uint32 V_0,
           int32 V_1)
  IL_0000:  ldarg.0
  IL_0001:  brfalse.s  IL_002c
  IL_0003:  ldc.i4     0x811c9dc5
  IL_0008:  stloc.0
  IL_0009:  ldc.i4.0
  IL_000a:  stloc.1
  IL_000b:  br.s       IL_0021
  IL_000d:  ldarg.0
  IL_000e:  ldloc.1
  IL_000f:  callvirt   instance char [mscorlib]System.String::get_Chars(int32)
  IL_0014:  ldloc.0
  IL_0015:  xor
  IL_0016:  ldc.i4     0x1000193
  IL_001b:  mul
  IL_001c:  stloc.0
  IL_001d:  ldloc.1
  IL_001e:  ldc.i4.1
  IL_001f:  add
  IL_0020:  stloc.1
  IL_0021:  ldloc.1
  IL_0022:  ldarg.0
  IL_0023:  callvirt   instance int32 [mscorlib]System.String::get_Length()
  IL_0028:  bge.s      IL_002c
  IL_002a:  br.s       IL_000d
  IL_002c:  ldloc.0
  IL_002d:  ret
} // end of method '<PrivateImplementationDetails>'::ComputeStringHash

JSIL strips <PrivateImplementationDetails> on translation.

[kg]

JSIL strips <PrivateImplementationDetails> on translation.

Ah, that sounds like a bug. Something that should probably be handled by DCE instead, I expect.

[iskiselev]

I suppose there is some filters to strip such classes. Previously it was used only for array initillization. At the same time it may be problem introduced by ILSpy- I was unable to find this class in ILSpy, it is visible only with ILDasm.

[iskiselev]

Here is hash function decompiled with dotPeek:

  internal static uint ComputeStringHash(string s)
  {
    uint num;
    if (s != null)
    {
      num = 2166136261U;
      for (int index = 0; index < s.Length; ++index)
        num = (uint) (((int) s[index] ^ (int) num) * 16777619);
    }
    return num;
  }

[kg]

Looks like it's always the same method body: http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.CSharp/Compiler/MethodBodySynthesizer.Lowered.cs,c6390aa114fbd516,references

[iskiselev]

Main problem - it may be dependent on compiler version. Nothing preventsRosly to change this implementation in future. For now we could as workaround add it to libraries and redirect usage of this method to our libraries, but how we can protect from future changes in that method? I suppose there will be only one such method per assembly, and we should preserve it - or found how we could restore switches back.

[kg]

I would preserve the method, it's the simplest solution. Just need to figure out why it's being pruned.