Browse Source

Fix #3382: Support compiler-generated throw-helper invocations in switch-expression implicit default-case.

Assisted-by: Claude:claude-fable-5:Claude Code
pull/3784/head
Siegfried Pammer 6 months ago
parent
commit
63757ead3b
  1. 22
      ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs
  2. 170
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/SwitchExpressions.cs
  3. 11
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs
  4. 146
      ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs
  5. 8
      ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs
  6. 38
      ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs

22
ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@ -141,6 +141,20 @@ namespace ICSharpCode.Decompiler.Tests @@ -141,6 +141,20 @@ namespace ICSharpCode.Decompiler.Tests
CompilerOptions.Optimize | CompilerOptions.UseRoslynLatest,
});
static readonly CompilerOptions[] roslyn3OrNewerWithNet40Roslyn4Options = Tester.SupportedOnCurrentPlatform(new[]
{
CompilerOptions.UseRoslyn4_14_0 | CompilerOptions.TargetNet40,
CompilerOptions.Optimize | CompilerOptions.UseRoslyn4_14_0 | CompilerOptions.TargetNet40,
CompilerOptions.UseRoslynLatest | CompilerOptions.TargetNet40,
CompilerOptions.Optimize | CompilerOptions.UseRoslynLatest | CompilerOptions.TargetNet40,
CompilerOptions.UseRoslyn3_11_0,
CompilerOptions.Optimize | CompilerOptions.UseRoslyn3_11_0,
CompilerOptions.UseRoslyn4_14_0,
CompilerOptions.Optimize | CompilerOptions.UseRoslyn4_14_0,
CompilerOptions.UseRoslynLatest,
CompilerOptions.Optimize | CompilerOptions.UseRoslynLatest,
});
static readonly CompilerOptions[] roslyn3OrNewerOptions = Tester.SupportedOnCurrentPlatform(new[]
{
CompilerOptions.UseRoslyn3_11_0,
@ -291,8 +305,14 @@ namespace ICSharpCode.Decompiler.Tests @@ -291,8 +305,14 @@ namespace ICSharpCode.Decompiler.Tests
});
}
// Runs on Roslyn 3.x and Roslyn 4.x or newer, with TargetNet40 variants only for
// Roslyn 4.x or newer: targeting net40 makes the compiler emit the
// ThrowInvalidOperationException throw helper (SwitchExpressionException does not
// exist there), but Roslyn 3.x emits a plain inline
// "throw new InvalidOperationException()" instead, which is indistinguishable from
// user code and therefore intentionally not transformed.
[Test]
public async Task SwitchExpressions([ValueSource(nameof(roslyn3OrNewerOptions))] CompilerOptions cscOptions)
public async Task SwitchExpressions([ValueSource(nameof(roslyn3OrNewerWithNet40Roslyn4Options))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions);
}

170
ICSharpCode.Decompiler.Tests/TestCases/Pretty/SwitchExpressions.cs

@ -159,5 +159,175 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -159,5 +159,175 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
_ => "default",
};
}
#pragma warning disable CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
public static int Issue3382(StringComparison c)
{
return c switch {
StringComparison.Ordinal => 0,
StringComparison.OrdinalIgnoreCase => 1,
};
}
public static void Issue3382b(ref StringComparison? c)
{
#if NET40
c = c switch {
null => StringComparison.Ordinal,
StringComparison.Ordinal => StringComparison.OrdinalIgnoreCase,
StringComparison.OrdinalIgnoreCase => StringComparison.InvariantCulture,
};
#else
StringComparison? stringComparison = c;
c = stringComparison switch {
null => StringComparison.Ordinal,
StringComparison.Ordinal => StringComparison.OrdinalIgnoreCase,
StringComparison.OrdinalIgnoreCase => StringComparison.InvariantCulture,
};
#endif
}
public static void Issue3382c(StringComparison? c)
{
c = c switch {
null => StringComparison.Ordinal,
StringComparison.Ordinal => StringComparison.OrdinalIgnoreCase,
StringComparison.OrdinalIgnoreCase => StringComparison.InvariantCulture,
};
}
public static void Issue3382d(ref StringComparison c)
{
#if NET40
c = c switch {
StringComparison.Ordinal => StringComparison.OrdinalIgnoreCase,
StringComparison.OrdinalIgnoreCase => StringComparison.InvariantCulture,
};
#else
StringComparison stringComparison = c;
c = stringComparison switch {
StringComparison.Ordinal => StringComparison.OrdinalIgnoreCase,
StringComparison.OrdinalIgnoreCase => StringComparison.InvariantCulture,
};
#endif
}
public static int SwitchOnStringImplicitDefault(string s)
{
return s switch {
"Hello" => 42,
"World" => 4711,
"!" => 7,
"Foo" => 13,
"Bar" => 21,
"Baz" => 84,
"Qux" => 168,
"Quux" => 336,
"Corge" => 672,
"Grault" => 1344,
"Garply" => 2688,
};
}
public static int SwitchOnStringImplicitDefaultFewCases(string s)
{
return s switch {
"red" => 1,
"green" => 2,
"blue" => 3,
};
}
public static int SwitchOnStringImplicitDefaultUniqueLengths(string s)
{
return s switch {
"a" => 1,
"bb" => 2,
"ccc" => 3,
"dddd" => 4,
"eeeee" => 5,
"ffffff" => 6,
"ggggggg" => 7,
"hhhhhhhh" => 8,
"iiiiiiiii" => 9,
};
}
public static int SwitchOnStringImplicitDefaultSameLength(string s)
{
return s switch {
"aabb" => 1,
"abab" => 2,
"abba" => 3,
"baab" => 4,
"baba" => 5,
"bbaa" => 6,
"bbbb" => 7,
"aaab" => 8,
"aaba" => 9,
};
}
public static int SwitchOnStringImplicitDefaultWithNullCase(string s)
{
return s switch {
"a" => 1,
"bb" => 2,
"ccc" => 3,
"dddd" => 4,
"eeeee" => 5,
"ffffff" => 6,
"ggggggg" => 7,
"hhhhhhhh" => 8,
"iiiiiiiii" => 9,
null => -1,
};
}
public static int SwitchOnStringImplicitDefaultTwoCases(string s)
{
return s switch {
"red" => 1,
"green" => 2,
};
}
public static string SwitchOnCharImplicitDefault(char c)
{
return c switch {
'a' => "first",
'b' => "second",
'c' => "third",
};
}
public static string SwitchOnLongImplicitDefault(long l)
{
return l switch {
1L => "one",
100L => "hundred",
10000L => "ten thousand",
long.MaxValue => "max",
};
}
public static void SwitchExpressionAsArgumentImplicitDefault(int i)
{
Console.WriteLine(i switch {
0 => "zero",
5 => "five",
10 => "ten",
});
}
public static int SwitchExpressionNestedImplicitDefault(StringComparison c, int i)
{
return c switch {
StringComparison.Ordinal => i switch {
0 => 1,
1 => 2,
},
StringComparison.OrdinalIgnoreCase => 3,
};
}
#pragma warning restore CS8509 // The switch expression does not handle all possible values of its input type (it is not exhaustive).
}
}

11
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -4135,10 +4135,13 @@ namespace ICSharpCode.Decompiler.CSharp @@ -4135,10 +4135,13 @@ namespace ICSharpCode.Decompiler.CSharp
switchExpr.SwitchSections.Add(ses);
}
var defaultSES = new SwitchExpressionSection();
defaultSES.Pattern = new IdentifierExpression("_");
defaultSES.Body = TranslateSectionBody(defaultSection);
switchExpr.SwitchSections.Add(defaultSES);
if (!defaultSection.IsCompilerGeneratedDefaultSection)
{
var defaultSES = new SwitchExpressionSection();
defaultSES.Pattern = new IdentifierExpression("_");
defaultSES.Body = TranslateSectionBody(defaultSection);
switchExpr.SwitchSections.Add(defaultSES);
}
return switchExpr.WithILInstruction(inst).WithRR(new ResolveResult(resultType));

146
ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs

@ -16,8 +16,10 @@ @@ -16,8 +16,10 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using ICSharpCode.Decompiler.FlowAnalysis;
@ -216,12 +218,150 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -216,12 +218,150 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
// (1st pass was in ControlFlowSimplification)
SimplifySwitchInstruction(block, context);
}
InlineSwitchExpressionDefaultCaseThrowHelper(block, context);
}
// call ThrowInvalidOperationException(...)
// leave IL_0000 (ldloc temp)
//
// -or-
//
// call ThrowSwitchExpressionException(...)
// leave IL_0000 (ldloc temp)
//
// -to-
//
// throw(newobj SwitchExpressionException(...))
internal static void InlineSwitchExpressionDefaultCaseThrowHelper(Block block, ILTransformContext context)
{
#nullable enable
// due to our use of basic blocks at this point,
// switch instructions can only appear as last instruction
var sw = block.Instructions.LastOrDefault() as SwitchInstruction;
if (sw == null)
return;
IMethod?[] exceptionCtorTable = new IMethod?[2];
exceptionCtorTable[0] = FindConstructor("System.InvalidOperationException");
exceptionCtorTable[1] = FindConstructor("System.Runtime.CompilerServices.SwitchExpressionException", typeof(object));
if (exceptionCtorTable[0] == null && exceptionCtorTable[1] == null)
return;
if (sw.GetDefaultSection() is not { Body: Branch { TargetBlock: Block defaultBlock } } defaultSection)
return;
if (defaultBlock is { Instructions: [var call, Branch or Leave] })
{
if (!MatchThrowHelperCall(call, out IMethod? exceptionCtor, out ILInstruction? value))
return;
context.Step("SwitchExpressionDefaultCaseTransform", block.Instructions[0]);
var newObj = new NewObj(exceptionCtor);
if (value != null)
newObj.Arguments.Add(value);
defaultBlock.Instructions[0] = new Throw(newObj).WithILRange(defaultBlock.Instructions[0]).WithILRange(defaultBlock.Instructions[1]);
defaultBlock.Instructions.RemoveAt(1);
defaultSection.IsCompilerGeneratedDefaultSection = true;
}
else if (defaultBlock is { Instructions: [Throw { Argument: NewObj { Method: var ctor, Arguments: [_] } }] }
&& ctor.Equals(exceptionCtorTable[1]))
{
defaultSection.IsCompilerGeneratedDefaultSection = true;
}
else
{
return;
}
bool MatchThrowHelperCall(ILInstruction inst, [NotNullWhen(true)] out IMethod? exceptionCtor, out ILInstruction? value)
{
exceptionCtor = null;
if (!MatchSwitchExpressionThrowHelperCall(inst, out value))
return false;
exceptionCtor = value == null ? exceptionCtorTable[0] : exceptionCtorTable[1];
return exceptionCtor != null;
}
IMethod? FindConstructor(string fullTypeName, params Type[] argumentTypes)
{
IType exceptionType = context.TypeSystem.FindType(new FullTypeName(fullTypeName));
var types = argumentTypes.SelectArray(context.TypeSystem.FindType);
foreach (var ctor in exceptionType.GetConstructors(m => !m.IsStatic && m.Parameters.Count == argumentTypes.Length))
{
bool found = true;
foreach (var pair in ctor.Parameters.Select(p => p.Type).Zip(types))
{
if (!NormalizeTypeVisitor.IgnoreNullability.EquivalentTypes(pair.Item1, pair.Item2))
{
found = false;
break;
}
}
if (found)
return ctor;
}
return null;
}
#nullable restore
}
#nullable enable
/// <summary>
/// Matches a call to one of the compiler-generated throw helpers in
/// &lt;PrivateImplementationDetails&gt; that implement the implicit default case of a
/// switch expression. <paramref name="value"/> is the switch value passed to
/// ThrowSwitchExpressionException, or null for the parameterless
/// ThrowInvalidOperationException helper (emitted when SwitchExpressionException
/// is not available on the target framework).
/// </summary>
static bool MatchSwitchExpressionThrowHelperCall(ILInstruction inst, out ILInstruction? value)
{
value = null;
if (inst is not Call call)
return false;
if (call.Method.DeclaringType.FullName != "<PrivateImplementationDetails>")
return false;
switch (call.Arguments.Count)
{
case 0:
return call.Method.Name == "ThrowInvalidOperationException";
case 1:
if (call.Method.Name != "ThrowSwitchExpressionException")
return false;
value = call.Arguments[0];
return true;
default:
return false;
}
}
/// <summary>
/// Returns true if the block consists solely of the compiler-generated default case
/// of a switch expression: either a throw-helper call followed by the unreachable
/// branch/leave to the end of the expression, or a directly inlined
/// "throw new SwitchExpressionException(...)" (emitted by older Roslyn versions).
/// </summary>
internal static bool IsSwitchExpressionThrowHelperBlock(Block block)
{
switch (block.Instructions)
{
case [var call, Branch or Leave]:
return MatchSwitchExpressionThrowHelperCall(call, out _);
case [Throw { Argument: NewObj { Method.DeclaringType: var exceptionType } }]:
return exceptionType.FullName == "System.Runtime.CompilerServices.SwitchExpressionException";
default:
return false;
}
}
#nullable restore
internal static void SimplifySwitchInstruction(Block block, ILTransformContext context)
{
// due to our of of basic blocks at this point,
// switch instructions can only appear as last insturction
// due to our use of basic blocks at this point,
// switch instructions can only appear as last instruction
var sw = block.Instructions.LastOrDefault() as SwitchInstruction;
if (sw == null)
return;
@ -452,7 +592,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -452,7 +592,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
!nullableBlock.Instructions.SecondToLastOrDefault().MatchIfInstruction(out var cond, out var trueInst) ||
!cond.MatchLogicNot(out var getHasValue) ||
!NullableLiftingTransform.MatchHasValueCall(getHasValue, out ILInstruction nullableInst))
{
return;
}
// could check that nullableInst is ldloc or ldloca and that the switch variable matches a GetValueOrDefault
// but the effect of adding an incorrect block to the flowBlock list would only be disasterous if it branched directly

8
ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs

@ -202,6 +202,12 @@ namespace ICSharpCode.Decompiler.IL @@ -202,6 +202,12 @@ namespace ICSharpCode.Decompiler.IL
/// </summary>
public bool HasNullLabel { get; set; }
/// <summary>
/// If true, this section only contains a compiler-generated throw helper
/// used in a switch expression and will not be visible in the decompiled source code.
/// </summary>
public bool IsCompilerGeneratedDefaultSection { get; set; }
/// <summary>
/// The set of labels that cause execution to jump to this switch section.
/// </summary>
@ -221,6 +227,8 @@ namespace ICSharpCode.Decompiler.IL @@ -221,6 +227,8 @@ namespace ICSharpCode.Decompiler.IL
public override void WriteTo(ITextOutput output, ILAstWritingOptions options)
{
WriteILRange(output, options);
if (IsCompilerGeneratedDefaultSection)
output.Write("generated.");
output.WriteLocalReference("case", this, isDefinition: true);
output.Write(' ');
if (HasNullLabel)

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

@ -88,6 +88,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -88,6 +88,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
if (!changed)
continue;
SwitchDetection.SimplifySwitchInstruction(block, context);
SwitchDetection.InlineSwitchExpressionDefaultCaseThrowHelper(block, context);
if (block.Parent is BlockContainer container)
changedContainers.Add(container);
}
@ -330,9 +331,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -330,9 +331,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms
currentCaseBlock = nextCaseBlock as Block;
} while (currentCaseBlock != null);
// We didn't find enough cases, exit
if (values.Count < 3)
// Short if-chains are more plausibly hand-written than a switch, so we require
// at least 3 cases - unless the chain ends in a compiler-generated throw helper,
// which proves that the source was a switch expression with an implicit default case.
if (values.Count < 3
&& !(currentCaseBlock != null && SwitchDetection.IsSwitchExpressionThrowHelperBlock(currentCaseBlock)))
{
return false;
}
context.Step(nameof(SimplifyCascadingIfStatements), instructions[i]);
// if the switchValueVar is used in other places as well, do not eliminate the store.
if (switchValueVar.LoadCount > numberOfUniqueMatchesWithCurrentVariable || !ValidateUsesOfSwitchValueVariable(switchValueVar, caseBlocks))
@ -1169,7 +1175,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1169,7 +1175,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms
}
var newSwitch = new SwitchInstruction(new StringToInt(switchValueInst, values, switchValueLoad.Variable.Type));
newSwitch.Sections.AddRange(sections);
newSwitch.Sections.Add(new SwitchSection { Labels = defaultLabel, Body = defaultSection.Body });
newSwitch.Sections.Add(new SwitchSection {
Labels = defaultLabel,
Body = defaultSection.Body,
IsCompilerGeneratedDefaultSection = defaultSection.IsCompilerGeneratedDefaultSection
});
instructions[offset].ReplaceWith(newSwitch);
return newSwitch;
}
@ -1217,6 +1227,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1217,6 +1227,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
switchOnLengthBlockStartOffset = i;
}
ILInstruction defaultCase = null;
bool defaultIsCompilerGenerated = false;
if (!MatchSwitchOnLengthBlock(ref switchValueVar, switchOnLengthBlock, switchOnLengthBlockStartOffset, out var blocksByLength))
return false;
List<(string, ILInstruction)> stringValues = new();
@ -1302,7 +1313,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1302,7 +1313,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms
}
var newSwitch = new SwitchInstruction(new StringToInt(new LdLoc(switchValueVar), values, switchValueVar.Type));
newSwitch.Sections.AddRange(sections);
newSwitch.Sections.Add(new SwitchSection { Labels = defaultLabel, Body = defaultCase is Block b2 ? new Branch(b2) : defaultCase });
newSwitch.Sections.Add(new SwitchSection {
Labels = defaultLabel,
Body = defaultCase is Block b2 ? new Branch(b2) : defaultCase,
IsCompilerGeneratedDefaultSection = defaultIsCompilerGenerated
});
newSwitch.AddILRange(instructions[i]);
if (nullCase != null)
{
@ -1312,6 +1327,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1312,6 +1327,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms
instructions.RemoveRange(i + 1, instructions.Count - (i + 1));
return true;
void InheritCompilerGeneratedDefaultMarker(SwitchInstruction inner)
{
// The char switches and the switch on the string length share the throw-helper
// block of the implicit default case; whichever of them was processed first by
// SwitchDetection carries the compiler-generated marker for it.
defaultIsCompilerGenerated |= inner.Sections.Any(s => s.IsCompilerGeneratedDefaultSection
&& s.Body.MatchBranch(out var target) && target == defaultCase);
}
bool MatchGetChars(ILInstruction instruction, ILVariable switchValueVar, out int index)
{
index = -1;
@ -1350,6 +1374,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1350,6 +1374,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return false;
if (!MatchGetChars(@switch.Value, switchValueVar, out index))
return false;
InheritCompilerGeneratedDefaultMarker(@switch);
sections = @switch.Sections.SelectList(s => new KeyValuePair<LongSet, ILInstruction>(s.Labels, s.Body));
break;
case 2:
@ -1364,6 +1389,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1364,6 +1389,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return false;
if (!@switch.Value.MatchLdLoc(charTempVar))
return false;
InheritCompilerGeneratedDefaultMarker(@switch);
sections = @switch.Sections.SelectList(s => new KeyValuePair<LongSet, ILInstruction>(s.Labels, s.Body));
break;
default:
@ -1514,6 +1540,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -1514,6 +1540,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
defaultCase ??= targetInst;
if (defaultCase != targetInst)
return false;
// The default section of the switch on the string length carries the
// compiler-generated marker if an implicit-default throw helper was
// already inlined there; transfer it to the rebuilt switch.
defaultIsCompilerGenerated |= section.IsCompilerGeneratedDefaultSection;
}
else
{

Loading…
Cancel
Save