Skip to content

Commit e1e2f73

Browse files
siegfriedpammersiegfriedpammer
committed
Fix switch-on-string transform for optimized Roslyn.
1 parent e4285b7 commit e1e2f73

File tree

2 files changed

+148
-41
lines changed

2 files changed

+148
-41
lines changed

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

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,46 @@ public static implicit operator int(ImplicitInt v)
8282
}
8383
}
8484

85+
public class ImplicitConversionConflictWithLong
86+
{
87+
private readonly int s;
88+
89+
public ImplicitConversionConflictWithLong(int s)
90+
{
91+
this.s = s;
92+
}
93+
94+
public static implicit operator int(ImplicitConversionConflictWithLong v)
95+
{
96+
return v.s;
97+
}
98+
99+
public static implicit operator long(ImplicitConversionConflictWithLong v)
100+
{
101+
return v.s;
102+
}
103+
}
104+
105+
public class ImplicitConversionConflictWithString
106+
{
107+
private readonly int s;
108+
109+
public ImplicitConversionConflictWithString(int s)
110+
{
111+
this.s = s;
112+
}
113+
114+
public static implicit operator int(ImplicitConversionConflictWithString v)
115+
{
116+
return v.s;
117+
}
118+
119+
public static implicit operator string(ImplicitConversionConflictWithString v)
120+
{
121+
return string.Empty;
122+
}
123+
}
124+
85125
public class ExplicitInt
86126
{
87127
private readonly int s;
@@ -396,6 +436,62 @@ public static void SwitchOverImplicitInt(ImplicitInt i)
396436
}
397437
}
398438

439+
public static void SwitchOverImplicitIntConflictLong(ImplicitConversionConflictWithLong i)
440+
{
441+
switch ((int)i)
442+
{
443+
case 0:
444+
Console.WriteLine("zero");
445+
break;
446+
case 5:
447+
Console.WriteLine("five");
448+
break;
449+
case 10:
450+
Console.WriteLine("ten");
451+
break;
452+
case 15:
453+
Console.WriteLine("fifteen");
454+
break;
455+
case 20:
456+
Console.WriteLine("twenty");
457+
break;
458+
case 25:
459+
Console.WriteLine("twenty-five");
460+
break;
461+
case 30:
462+
Console.WriteLine("thirty");
463+
break;
464+
}
465+
}
466+
467+
public static void SwitchOverImplicitIntConflictString(ImplicitConversionConflictWithString i)
468+
{
469+
switch ((string)i)
470+
{
471+
case "0":
472+
Console.WriteLine("zero");
473+
break;
474+
case "5":
475+
Console.WriteLine("five");
476+
break;
477+
case "10":
478+
Console.WriteLine("ten");
479+
break;
480+
case "15":
481+
Console.WriteLine("fifteen");
482+
break;
483+
case "20":
484+
Console.WriteLine("twenty");
485+
break;
486+
case "25":
487+
Console.WriteLine("twenty-five");
488+
break;
489+
case "30":
490+
Console.WriteLine("thirty");
491+
break;
492+
}
493+
}
494+
399495
// SwitchDetection.UseCSharpSwitch requires more complex heuristic to identify this when compiled with Roslyn
400496
public static void CompactSwitchOverInt(int i)
401497
{
@@ -507,9 +603,7 @@ public static string SwitchOverString2()
507603

508604
public static string SwitchOverImplicitString(ImplicitString s)
509605
{
510-
// we emit an explicit cast, because the rules used by the C# compiler are counter-intuitive:
511-
// The C# compiler does *not* take the type of the switch labels into account at all.
512-
switch ((string)s)
606+
switch (s)
513607
{
514608
case "First case":
515609
return "Text1";

‎ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs‎

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,10 +1070,12 @@ bool MatchRoslynSwitchOnString(InstructionCollection<ILInstruction> instructions
10701070
stringValues.Add((null, nullValueCaseBlock));
10711071
}
10721072
// In newer Roslyn versions (>=3.7) the null check appears in the default case, not prior to the switch.
1073-
if (!stringValues.Any(pair => pair.Value == null) && IsNullCheckInDefaultBlock(ref exitOrDefaultBlock, switchValueLoad.Variable, out nullValueCaseBlock))
1073+
ILInstruction exitOrDefault = exitOrDefaultBlock;
1074+
if (!stringValues.Any(pair => pair.Value == null) && IsNullCheckInDefaultBlock(ref exitOrDefault, switchValueLoad.Variable, out nullValueCaseBlock))
10741075
{
10751076
stringValues.Add((null, nullValueCaseBlock));
10761077
}
1078+
exitOrDefaultBlock = (Block)exitOrDefault;
10771079

10781080
context.Step(nameof(MatchRoslynSwitchOnString), switchValueLoad);
10791081
if (exitOrDefaultBlock != null)
@@ -1176,7 +1178,7 @@ private bool MatchRoslynSwitchOnStringUsingLengthAndChar(Block block, int i)
11761178
{
11771179
if (!instructions[i + 1].MatchBranch(out var nextBlock))
11781180
return false;
1179-
if (!exitBlockJump.MatchBranch(out nullCase))
1181+
if (!exitBlockJump.MatchBranch(out nullCase) && !exitBlockJump.MatchLeave(out _))
11801182
return false;
11811183
// if (comp(ldloc switchValueVar == ldnull)) br ...
11821184
// br switchOnLengthBlock
@@ -1202,7 +1204,7 @@ private bool MatchRoslynSwitchOnStringUsingLengthAndChar(Block block, int i)
12021204
switchValueVar = null; // will be extracted in MatchSwitchOnLengthBlock
12031205
switchOnLengthBlockStartOffset = i;
12041206
}
1205-
Block defaultCase = null;
1207+
ILInstruction defaultCase = null;
12061208
if (!MatchSwitchOnLengthBlock(ref switchValueVar, switchOnLengthBlock, switchOnLengthBlockStartOffset, out var blocksByLength))
12071209
return false;
12081210
List<(string, ILInstruction)> stringValues = new();
@@ -1216,41 +1218,51 @@ private bool MatchRoslynSwitchOnStringUsingLengthAndChar(Block block, int i)
12161218
else
12171219
{
12181220
int length = (int)b.Length.Intervals[0].Start;
1219-
if (MatchSwitchOnCharBlock(b.TargetBlock, length, switchValueVar, out var mapping))
1221+
switch (b.TargetBlock)
12201222
{
1221-
foreach (var item in mapping)
1222-
{
1223-
if (!stringValues.Any(x => x.Item1 == item.StringValue))
1223+
case Leave leave:
1224+
break;
1225+
case Block targetBlock:
1226+
if (MatchSwitchOnCharBlock(targetBlock, length, switchValueVar, out var mapping))
1227+
{
1228+
foreach (var item in mapping)
1229+
{
1230+
if (!stringValues.Any(x => x.Item1 == item.StringValue))
1231+
{
1232+
stringValues.Add(item);
1233+
}
1234+
else
1235+
{
1236+
return false;
1237+
}
1238+
}
1239+
}
1240+
else if (MatchRoslynCaseBlockHead(targetBlock, switchValueVar, out var bodyOrLeave, out var exit, out string stringValue, out _))
1241+
{
1242+
if (exit != defaultCase)
1243+
return false;
1244+
if (!stringValues.Any(x => x.Item1 == stringValue))
1245+
{
1246+
stringValues.Add((stringValue, bodyOrLeave));
1247+
}
1248+
else
1249+
{
1250+
return false;
1251+
}
1252+
}
1253+
else if (length == 0)
12241254
{
1225-
stringValues.Add(item);
1255+
stringValues.Add(("", b.TargetBlock));
12261256
}
12271257
else
12281258
{
12291259
return false;
12301260
}
1231-
}
1232-
}
1233-
else if (MatchRoslynCaseBlockHead(b.TargetBlock, switchValueVar, out var bodyOrLeave, out var exit, out string stringValue, out _))
1234-
{
1235-
if (exit != defaultCase)
1261+
break;
1262+
default:
12361263
return false;
1237-
if (!stringValues.Any(x => x.Item1 == stringValue))
1238-
{
1239-
stringValues.Add((stringValue, bodyOrLeave));
1240-
}
1241-
else
1242-
{
1243-
return false;
1244-
}
1245-
}
1246-
else if (length == 0)
1247-
{
1248-
stringValues.Add(("", b.TargetBlock));
1249-
}
1250-
else
1251-
{
1252-
return false;
12531264
}
1265+
12541266
}
12551267
}
12561268

@@ -1278,7 +1290,7 @@ private bool MatchRoslynSwitchOnStringUsingLengthAndChar(Block block, int i)
12781290
}
12791291
var newSwitch = new SwitchInstruction(new StringToInt(new LdLoc(switchValueVar), values, switchValueVar.Type));
12801292
newSwitch.Sections.AddRange(sections);
1281-
newSwitch.Sections.Add(new SwitchSection { Labels = defaultLabel, Body = new Branch(defaultCase) });
1293+
newSwitch.Sections.Add(new SwitchSection { Labels = defaultLabel, Body = defaultCase is Block b2 ? new Branch(b2) : defaultCase });
12821294
newSwitch.AddILRange(instructions[i]);
12831295
if (nullCase != null)
12841296
{
@@ -1399,7 +1411,7 @@ bool MatchSwitchOnCharBlock(Block block, int length, ILVariable switchValueVar,
13991411
return results?.Count > 0;
14001412
}
14011413

1402-
bool MatchSwitchOnLengthBlock(ref ILVariable switchValueVar, Block switchOnLengthBlock, int startOffset, out List<(LongSet Length, Block TargetBlock)> blocks)
1414+
bool MatchSwitchOnLengthBlock(ref ILVariable switchValueVar, Block switchOnLengthBlock, int startOffset, out List<(LongSet Length, ILInstruction TargetBlock)> blocks)
14031415
{
14041416
blocks = null;
14051417
SwitchInstruction @switch;
@@ -1482,17 +1494,18 @@ bool MatchSwitchOnLengthBlock(ref ILVariable switchValueVar, Block switchOnLengt
14821494
{
14831495
if (section.HasNullLabel)
14841496
return false;
1485-
if (!section.Body.MatchBranch(out var target))
1497+
if (!section.Body.MatchBranch(out var target) && !section.Body.MatchLeave(out _))
14861498
return false;
1499+
ILInstruction targetInst = target ?? section.Body;
14871500
if (section.Labels.Count() != 1)
14881501
{
1489-
defaultCase ??= target;
1490-
if (defaultCase != target)
1502+
defaultCase ??= targetInst;
1503+
if (defaultCase != targetInst)
14911504
return false;
14921505
}
14931506
else
14941507
{
1495-
blocks.Add((section.Labels, target));
1508+
blocks.Add((section.Labels, targetInst));
14961509
}
14971510
}
14981511
return true;
@@ -1506,10 +1519,10 @@ bool MatchSwitchOnLengthBlock(ref ILVariable switchValueVar, Block switchOnLengt
15061519
/// br newDefaultBlock
15071520
/// }
15081521
/// </summary>
1509-
private bool IsNullCheckInDefaultBlock(ref Block exitOrDefaultBlock, ILVariable switchVar, out Block nullValueCaseBlock)
1522+
private bool IsNullCheckInDefaultBlock(ref ILInstruction exitOrDefault, ILVariable switchVar, out Block nullValueCaseBlock)
15101523
{
15111524
nullValueCaseBlock = null;
1512-
if (exitOrDefaultBlock == null)
1525+
if (exitOrDefault is not Block exitOrDefaultBlock)
15131526
return false;
15141527
if (!exitOrDefaultBlock.Instructions[0].MatchIfInstruction(out var condition, out var thenBranch))
15151528
return false;
@@ -1523,7 +1536,7 @@ private bool IsNullCheckInDefaultBlock(ref Block exitOrDefaultBlock, ILVariable
15231536
return false;
15241537
if (elseBlock.Parent != exitOrDefaultBlock.Parent)
15251538
return false;
1526-
exitOrDefaultBlock = elseBlock;
1539+
exitOrDefault = elseBlock;
15271540
return true;
15281541
}
15291542

0 commit comments

Comments
 (0)