Browse Source

Fix switch on nullable for Roslyn. Ignore switch on bool because it is indistinguishable from if (bool).

pull/1425/head
Siegfried Pammer 6 years ago
parent
commit
7671ac6fe4
  1. 1
      ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs
  2. 12
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs
  3. 28
      ICSharpCode.Decompiler/IL/Transforms/SwitchOnNullableTransform.cs

1
ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@ -119,7 +119,6 @@ namespace ICSharpCode.Decompiler.Tests @@ -119,7 +119,6 @@ namespace ICSharpCode.Decompiler.Tests
}
[Test]
[Ignore("broken by Roslyn upgrade")]
public void Switch([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
{
RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings {

12
ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs

@ -47,6 +47,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -47,6 +47,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Null
}
#if !ROSLYN
public static State SwitchOverNullableBool(bool? value)
{
switch (value) {
@ -60,6 +61,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -60,6 +61,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
throw new InvalidOperationException();
}
}
#endif
public static bool? SwitchOverNullableEnum(State? state)
{
@ -361,6 +363,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -361,6 +363,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
}
#if !ROSLYN
public static string SwitchOverBool(bool b)
{
Console.WriteLine("SwitchOverBool: " + b.ToString());
@ -373,6 +376,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -373,6 +376,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
return null;
}
}
#endif
public static void SwitchInLoop(int i)
{
@ -914,9 +918,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -914,9 +918,9 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
// These decompile poorly into switch statements and should be left as is
#region Overagressive Switch Use
#region Overagressive Switch Use
#if ROSLYN || OPT
#if ROSLYN || OPT
public static void SingleIf1(int i, bool a)
{
if (i == 1 || (i == 2 && a)) {
@ -924,7 +928,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -924,7 +928,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine(2);
}
#endif
#endif
public static void SingleIf2(int i, bool a, bool b)
{
@ -1067,7 +1071,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1067,7 +1071,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
c = getChar();
} while (c != -1 && c != '\n' && c != '\u2028' && c != '\u2029');
}
#endregion
#endregion
// Ensure correctness of SwitchDetection.UseCSharpSwitch control flow heuristics
public static void SwitchWithBreakCase(int i, bool b)

28
ICSharpCode.Decompiler/IL/Transforms/SwitchOnNullableTransform.cs

@ -50,10 +50,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -50,10 +50,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
continue;
}
if (MatchRoslynSwitchOnNullable(block.Instructions, i, out newSwitch)) {
newSwitch.AddILRange(block.Instructions[i - 1]);
block.Instructions[i - 1].ReplaceWith(newSwitch);
block.Instructions.RemoveRange(i, 2);
i--;
newSwitch.AddILRange(block.Instructions[i]);
newSwitch.AddILRange(block.Instructions[i + 1]);
block.Instructions[i].ReplaceWith(newSwitch);
block.Instructions.RemoveAt(i + 1);
changed = true;
continue;
}
@ -130,18 +130,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -130,18 +130,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms
{
newSwitch = null;
// match first block:
// stloc tmp(ldloc switchValueVar)
// if (logic.not(call get_HasValue(ldloca tmp))) br nullCaseBlock
// if (logic.not(call get_HasValue(target))) br nullCaseBlock
// br switchBlock
if (i < 1) return false;
if (!instructions[i - 1].MatchStLoc(out var tmp, out var switchValue) ||
!instructions[i].MatchIfInstruction(out var condition, out var trueInst))
return false;
if (tmp.StoreCount != 1 || tmp.AddressCount != 2 || tmp.LoadCount != 0)
if (!instructions[i].MatchIfInstruction(out var condition, out var trueInst))
return false;
if (!instructions[i + 1].MatchBranch(out var switchBlock) || !trueInst.MatchBranch(out var nullCaseBlock))
return false;
if (!condition.MatchLogicNot(out var getHasValue) || !NullableLiftingTransform.MatchHasValueCall(getHasValue, out ILVariable target1) || target1 != tmp)
if (!condition.MatchLogicNot(out var getHasValue) || !NullableLiftingTransform.MatchHasValueCall(getHasValue, out ILInstruction target) || !SemanticHelper.IsPure(target.Flags))
return false;
// match second block: switchBlock
// note: I have seen cases where switchVar is inlined into the switch.
@ -161,7 +156,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -161,7 +156,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return false;
if (!switchVar.IsSingleDefinition || switchVar.LoadCount != 1)
return false;
if (!NullableLiftingTransform.MatchGetValueOrDefault(getValueOrDefault, tmp))
if (!NullableLiftingTransform.MatchGetValueOrDefault(getValueOrDefault, out ILInstruction target2) && target2.Match(target).Success)
return false;
if (!(switchBlock.Instructions[1] is SwitchInstruction si))
return false;
@ -172,7 +167,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -172,7 +167,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
// this is the special case where `call GetValueOrDefault(ldloca tmp)` is inlined into the switch.
if (!(switchBlock.Instructions[0] is SwitchInstruction si))
return false;
if (!NullableLiftingTransform.MatchGetValueOrDefault(si.Value, tmp))
if (!NullableLiftingTransform.MatchGetValueOrDefault(si.Value, out ILInstruction target2) && target2.Match(target).Success)
return false;
switchInst = si;
break;
@ -181,6 +176,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -181,6 +176,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return false;
}
}
ILInstruction switchValue;
if (target.MatchLdLoca(out var v))
switchValue = new LdLoc(v).WithILRange(target);
else
switchValue = new LdObj(target, ((CallInstruction)getHasValue).Method.DeclaringType);
newSwitch = BuildLiftedSwitch(nullCaseBlock, switchInst, switchValue);
return true;
}

Loading…
Cancel
Save