diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index fc4a4f2b4..3edd86c90 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -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 }); } + // 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); } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/SwitchExpressions.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/SwitchExpressions.cs index dafe2f548..15cfcbbe6 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/SwitchExpressions.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/SwitchExpressions.cs @@ -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). } } \ No newline at end of file diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index ee7177f48..e256ec34e 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -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)); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs index edc2d43b9..738a21533 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs @@ -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 // (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 + /// + /// Matches a call to one of the compiler-generated throw helpers in + /// <PrivateImplementationDetails> that implement the implicit default case of a + /// switch expression. is the switch value passed to + /// ThrowSwitchExpressionException, or null for the parameterless + /// ThrowInvalidOperationException helper (emitted when SwitchExpressionException + /// is not available on the target framework). + /// + static bool MatchSwitchExpressionThrowHelperCall(ILInstruction inst, out ILInstruction? value) + { + value = null; + if (inst is not Call call) + return false; + if (call.Method.DeclaringType.FullName != "") + 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; + } + } + + /// + /// 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). + /// + 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 !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 diff --git a/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs index ad0d0a9a4..880062620 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/SwitchInstruction.cs @@ -202,6 +202,12 @@ namespace ICSharpCode.Decompiler.IL /// public bool HasNullLabel { get; set; } + /// + /// 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. + /// + public bool IsCompilerGeneratedDefaultSection { get; set; } + /// /// The set of labels that cause execution to jump to this switch section. /// @@ -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) diff --git a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs index 1d4bb18cb..88983bdcc 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs @@ -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 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 } 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 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 } 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 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 return false; if (!MatchGetChars(@switch.Value, switchValueVar, out index)) return false; + InheritCompilerGeneratedDefaultMarker(@switch); sections = @switch.Sections.SelectList(s => new KeyValuePair(s.Labels, s.Body)); break; case 2: @@ -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(s.Labels, s.Body)); break; default: @@ -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 {