From 3d8cda5f882ab04e86ee839e95efb3ffaa369165 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 4 Aug 2023 11:44:43 +0200 Subject: [PATCH] Fix #2613: Detect pattern matching on variables of generic type with value types. --- .../TestCases/Pretty/PatternMatching.cs | 72 ++++++++++++++++ .../CSharp/ExpressionBuilder.cs | 9 ++ .../IL/Transforms/PatternMatchingTransform.cs | 82 ++++++++++++++----- 3 files changed, 142 insertions(+), 21 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs index 064db32bcc..5b9f38787b 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/PatternMatching.cs @@ -231,6 +231,78 @@ public static void NotTypePatternBecauseVarIsNotDefAssignedInCaseOfFallthrough(o #endif } + public void GenericTypePatternInt(T x) + { + if (x is int value) + { + Console.WriteLine(value); + } + else + { + Console.WriteLine("not an int"); + } + } + + public void GenericValueTypePatternInt(T x) where T : struct + { + if (x is int value) + { + Console.WriteLine(value); + } + else + { + Console.WriteLine("not an int"); + } + } + + public void GenericRefTypePatternInt(T x) where T : class + { + if (x is int value) + { + Console.WriteLine(value); + } + else + { + Console.WriteLine("not an int"); + } + } + + public void GenericTypePatternString(T x) + { + if (x is string value) + { + Console.WriteLine(value); + } + else + { + Console.WriteLine("not a string"); + } + } + + public void GenericRefTypePatternString(T x) where T : class + { + if (x is string value) + { + Console.WriteLine(value); + } + else + { + Console.WriteLine("not a string"); + } + } + + public void GenericValueTypePatternStringRequiresCastToObject(T x) where T : struct + { + if ((object)x is string value) + { + Console.WriteLine(value); + } + else + { + Console.WriteLine("not a string"); + } + } + private bool F() { return true; diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 2001cbea11..707377b47d 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -4537,6 +4537,15 @@ void ReplaceAssignmentTarget(ILInstruction target) protected internal override TranslatedExpression VisitMatchInstruction(MatchInstruction inst, TranslationContext context) { var left = Translate(inst.TestedOperand); + // remove boxing conversion if possible, however, we still need a cast in + // test case PatternMatching.GenericValueTypePatternStringRequiresCastToObject + if (left.ResolveResult is ConversionResolveResult crr + && crr.Conversion.IsBoxingConversion + && left.Expression is CastExpression castExpr + && (crr.Input.Type.IsReferenceType != false || inst.Variable.Type.IsReferenceType == false)) + { + left = left.UnwrapChild(castExpr.Expression); + } var right = TranslatePattern(inst); return new BinaryOperatorExpression(left, BinaryOperatorType.IsPattern, right) diff --git a/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs index 97edacb66a..9a02d9c7e3 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/PatternMatchingTransform.cs @@ -240,21 +240,25 @@ private bool CheckAllUsesDominatedBy(ILVariable v, BlockContainer container, ILI /// } private bool PatternMatchValueTypes(Block block, BlockContainer container, ILTransformContext context, ref ControlFlowGraph? cfg) { - if (!MatchIsInstBlock(block, out var type, out var testedOperand, - out var unboxBlock, out var falseBlock)) + if (!MatchIsInstBlock(block, out var type, out var testedOperand, out var testedVariable, + out var boxType1, out var unboxBlock, out var falseBlock)) { return false; } StLoc? tempStore = block.Instructions.ElementAtOrDefault(block.Instructions.Count - 3) as StLoc; - if (tempStore == null || !tempStore.Value.MatchLdLoc(testedOperand.Variable)) + if (tempStore == null || !tempStore.Value.MatchLdLoc(testedVariable)) { tempStore = null; } - if (!MatchUnboxBlock(unboxBlock, type, out var unboxOperand, out var v, out var storeToV)) + if (!MatchUnboxBlock(unboxBlock, type, out var unboxOperand, out var boxType2, out var storeToV)) { return false; } - if (unboxOperand == testedOperand.Variable) + if (!object.Equals(boxType1, boxType2)) + { + return false; + } + if (unboxOperand == testedVariable) { // do nothing } @@ -267,11 +271,11 @@ private bool PatternMatchValueTypes(Block block, BlockContainer container, ILTra { return false; } - if (!CheckAllUsesDominatedBy(v, container, unboxBlock, storeToV, null, context, ref cfg)) + if (!CheckAllUsesDominatedBy(storeToV.Variable, container, unboxBlock, storeToV, null, context, ref cfg)) return false; - context.Step($"PatternMatching with {v.Name}", block); + context.Step($"PatternMatching with {storeToV.Variable.Name}", block); var ifInst = (IfInstruction)block.Instructions.SecondToLastOrDefault()!; - ifInst.Condition = new MatchInstruction(v, testedOperand) { + ifInst.Condition = new MatchInstruction(storeToV.Variable, testedOperand) { CheckNotNull = true, CheckType = true }; @@ -286,25 +290,35 @@ private bool PatternMatchValueTypes(Block block, BlockContainer container, ILTra // should become the then-branch. Change the unboxBlock StartILOffset from an offset inside // the pattern matching machinery to an offset belonging to an instruction in the then-block. unboxBlock.SetILRange(unboxBlock.Instructions[0]); - v.Kind = VariableKind.PatternLocal; + storeToV.Variable.Kind = VariableKind.PatternLocal; return true; } /// ... /// if (comp.o(isinst T(ldloc testedOperand) == ldnull)) br falseBlock /// br unboxBlock + /// - or - + /// ... + /// if (comp.o(isinst T(box ``0(ldloc testedOperand)) == ldnull)) br falseBlock + /// br unboxBlock private bool MatchIsInstBlock(Block block, [NotNullWhen(true)] out IType? type, - [NotNullWhen(true)] out LdLoc? testedOperand, + [NotNullWhen(true)] out ILInstruction? testedOperand, + [NotNullWhen(true)] out ILVariable? testedVariable, + out IType? boxType, [NotNullWhen(true)] out Block? unboxBlock, [NotNullWhen(true)] out Block? falseBlock) { type = null; testedOperand = null; + testedVariable = null; + boxType = null; unboxBlock = null; falseBlock = null; if (!block.MatchIfAtEndOfBlock(out var condition, out var trueInst, out var falseInst)) + { return false; + } if (condition.MatchCompEqualsNull(out var arg)) { ExtensionMethods.Swap(ref trueInst, ref falseInst); @@ -317,11 +331,18 @@ private bool MatchIsInstBlock(Block block, { return false; } - if (!arg.MatchIsInst(out arg, out type)) + if (!arg.MatchIsInst(out testedOperand, out type)) + { return false; - testedOperand = arg as LdLoc; - if (testedOperand == null) + } + if (!(testedOperand.MatchBox(out var boxArg, out boxType) && boxType.Kind == TypeKind.TypeParameter)) + { + boxArg = testedOperand; + } + if (!boxArg.MatchLdLoc(out testedVariable)) + { return false; + } return trueInst.MatchBranch(out unboxBlock) && falseInst.MatchBranch(out falseBlock) && unboxBlock.Parent == block.Parent && falseBlock.Parent == block.Parent; } @@ -329,21 +350,40 @@ private bool MatchIsInstBlock(Block block, /// Block unboxBlock (incoming: 1) { /// stloc V(unbox.any T(ldloc testedOperand)) /// ... + /// - or - + /// stloc V(unbox.any T(isinst T(box ``0(ldloc testedOperand)))) + /// ... /// } - private bool MatchUnboxBlock(Block unboxBlock, IType type, [NotNullWhen(true)] out ILVariable? testedOperand, - [NotNullWhen(true)] out ILVariable? v, [NotNullWhen(true)] out ILInstruction? storeToV) + private bool MatchUnboxBlock(Block unboxBlock, IType type, [NotNullWhen(true)] out ILVariable? testedVariable, + out IType? boxType, [NotNullWhen(true)] out StLoc? storeToV) { - v = null; + boxType = null; storeToV = null; - testedOperand = null; + testedVariable = null; if (unboxBlock.IncomingEdgeCount != 1) return false; - storeToV = unboxBlock.Instructions[0]; - if (!storeToV.MatchStLoc(out v, out var value)) + storeToV = unboxBlock.Instructions[0] as StLoc; + if (storeToV == null) return false; - if (!(value.MatchUnboxAny(out var arg, out var t) && t.Equals(type) && arg.MatchLdLoc(out testedOperand))) + var value = storeToV.Value; + if (!(value.MatchUnboxAny(out var arg, out var t) && t.Equals(type))) return false; - + if (arg.MatchIsInst(out var isinstArg, out var isinstType) && isinstType.Equals(type)) + { + arg = isinstArg; + } + if (arg.MatchBox(out var boxArg, out boxType) && boxType.Kind == TypeKind.TypeParameter) + { + arg = boxArg; + } + if (!arg.MatchLdLoc(out testedVariable)) + { + return false; + } + if (boxType != null && !boxType.Equals(testedVariable.Type)) + { + return false; + } return true; } }