From b435c07f4d7696849833988a8fefa00260a2f51e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 15 Aug 2020 15:44:03 +0200 Subject: [PATCH] Added support for deconstruction assignments to properties --- .../CorrectnessTestRunner.cs | 6 + .../ICSharpCode.Decompiler.Tests.csproj | 1 + .../Correctness/DeconstructionTests.cs | 131 ++++++++++++++++ .../TestCases/Pretty/DeconstructionTests.cs | 30 +++- .../CSharp/ExpressionBuilder.cs | 14 ++ ICSharpCode.Decompiler/IL/ILVariable.cs | 7 + .../IL/Instructions/DeconstructInstruction.cs | 29 ++-- .../IL/Instructions/ILInstruction.cs | 80 ++++++++++ .../IL/Transforms/DeconstructionTransform.cs | 146 +++++++++++++----- .../IL/Transforms/ILInlining.cs | 14 ++ ILSpy/Properties/Resources.resx | 6 +- 11 files changed, 415 insertions(+), 49 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs diff --git a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs index ebd186188..ac9c66bf6 100644 --- a/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs @@ -202,6 +202,12 @@ namespace ICSharpCode.Decompiler.Tests RunCS(options: options); } + [Test] + public void DeconstructionTests([ValueSource("roslynOnlyOptions")] CompilerOptions options) + { + RunCS(options: options); + } + [Test] public void BitNot([Values(false, true)] bool force32Bit) { diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index f900e1273..93cbbe251 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -91,6 +91,7 @@ + diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs new file mode 100644 index 000000000..962ae7121 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/DeconstructionTests.cs @@ -0,0 +1,131 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness +{ + class DeconstructionTests + { + public static void Main() + { + new CustomDeconstructionAndConversion().Test(); + } + + private class CustomDeconstructionAndConversion + { + public struct MyInt + { + public static implicit operator int(MyInt x) + { + return 0; + } + + public static implicit operator MyInt(int x) + { + return default(MyInt); + } + } + + public int IntField; + + public int? NullableIntField; + + public MyInt MyIntField; + + public MyInt? NullableMyIntField; + private MyInt? nMy; + private MyInt my; + + public int Int { + get; + set; + } + + public int? NInt { + get; + set; + } + + public MyInt My { + get { + Console.WriteLine("get_My"); + return my; + } + set { + Console.WriteLine("set_My"); + my = value; + } + } + + public MyInt? NMy { + get { + Console.WriteLine("get_NMy"); + return nMy; + } + set { + Console.WriteLine("set_NMy"); + nMy = value; + } + } + + public static MyInt StaticMy { + get; + set; + } + + public static MyInt? StaticNMy { + get; + set; + } + + public void Deconstruct(out MyInt? x, out MyInt y) + { + Console.WriteLine("Deconstruct(x, y)"); + x = null; + y = default(MyInt); + } + + public CustomDeconstructionAndConversion GetValue() + { + Console.WriteLine($"GetValue()"); + return new CustomDeconstructionAndConversion(); + } + + public CustomDeconstructionAndConversion Get(int i) + { + Console.WriteLine($"Get({i})"); + return new CustomDeconstructionAndConversion(); + } + + private MyInt? GetNullableMyInt() + { + throw new NotImplementedException(); + } + + public void Test() + { + Property_NoDeconstruction_SwappedAssignments(); + Property_NoDeconstruction_SwappedInits(); + } + + public void Property_NoDeconstruction_SwappedAssignments() + { + Console.WriteLine("Property_NoDeconstruction_SwappedAssignments:"); + CustomDeconstructionAndConversion customDeconstructionAndConversion = Get(0); + CustomDeconstructionAndConversion customDeconstructionAndConversion2 = Get(1); + GetValue().Deconstruct(out MyInt? x, out MyInt y); + MyInt myInt2 = customDeconstructionAndConversion2.My = y; + MyInt? myInt4 = customDeconstructionAndConversion.NMy = x; + } + + public void Property_NoDeconstruction_SwappedInits() + { + Console.WriteLine("Property_NoDeconstruction_SwappedInits:"); + CustomDeconstructionAndConversion customDeconstructionAndConversion = Get(1); + (Get(0).NMy, customDeconstructionAndConversion.My) = GetValue(); + } + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs index 8d834ef7b..40c52826d 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/DeconstructionTests.cs @@ -67,6 +67,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty set; } + public static MyInt StaticMy { + get; + set; + } + + public static MyInt? StaticNMy { + get; + set; + } + public void Deconstruct(out MyInt? x, out MyInt y) { x = null; @@ -88,7 +98,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty throw new NotImplementedException(); } - public void Test7() + public void LocalVariable_NoConversion() { MyInt? myInt3; MyInt x; @@ -96,6 +106,24 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty Console.WriteLine(myInt3); Console.WriteLine(x); } + + public void LocalVariable_NoConversion_ComplexValue() + { + MyInt? myInt3; + MyInt x; + (myInt3, x) = new CustomDeconstructionAndConversion { + My = 3 + }; + Console.WriteLine(myInt3); + Console.WriteLine(x); + } + + public void Property_NoConversion() + { + (Get(0).NMy, Get(1).My) = GetValue(); + } + + } } } diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 2437a89e6..ce10b9e90 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -3390,6 +3390,8 @@ namespace ICSharpCode.Decompiler.CSharp rhs = rhs.ConvertTo(rhsType, this); // TODO allowImplicitConversion var assignments = inst.Assignments.Instructions; int assignmentPos = 0; + var inits = inst.Init; + int initPos = 0; var lhs = ConstructTuple(inst.Pattern); return new AssignmentExpression(lhs, rhs) .WithILInstruction(inst) @@ -3418,6 +3420,18 @@ namespace ICSharpCode.Decompiler.CSharp case StLoc stloc: Debug.Assert(stloc.Value.MatchLdLoc(value)); break; + case CallInstruction call: + for (int i = 0; i < call.Arguments.Count - 1; i++) { + if (call.Arguments[i].MatchLdLoc(out var v) + && v.Kind == VariableKind.DeconstructionInitTemporary) + { + Debug.Assert(inits[initPos].Variable == v); + call.Arguments[i] = inits[initPos].Value; + initPos++; + } + } + Debug.Assert(call.Arguments.Last().MatchLdLoc(value)); + break; default: throw new NotSupportedException(); } diff --git a/ICSharpCode.Decompiler/IL/ILVariable.cs b/ICSharpCode.Decompiler/IL/ILVariable.cs index 691119e79..fb833467d 100644 --- a/ICSharpCode.Decompiler/IL/ILVariable.cs +++ b/ICSharpCode.Decompiler/IL/ILVariable.cs @@ -79,6 +79,10 @@ namespace ICSharpCode.Decompiler.IL /// Local variable declared within a pattern match. /// PatternLocal, + /// + /// Temporary variable declared in a deconstruction init section. + /// + DeconstructionInitTemporary, } static class VariableKindExtensions @@ -440,6 +444,9 @@ namespace ICSharpCode.Decompiler.IL case VariableKind.PatternLocal: output.Write("pattern local "); break; + case VariableKind.DeconstructionInitTemporary: + output.Write("deconstruction init temporary "); + break; default: throw new ArgumentOutOfRangeException(); } diff --git a/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs index 6e65153ee..679c8a6f6 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/DeconstructInstruction.cs @@ -172,7 +172,7 @@ namespace ICSharpCode.Decompiler.IL pattern.WriteTo(output, options); output.Unindent(); output.WriteLine(); - output.Write("conversions:"); + output.Write("conversions: "); conversions.WriteTo(output, options); output.WriteLine(); output.Write("assignments: "); @@ -202,24 +202,33 @@ namespace ICSharpCode.Decompiler.IL return input.MatchLdLoc(out inputVariable) || input.MatchLdLoca(out inputVariable); } - internal static bool IsAssignment(ILInstruction inst, out ILVariable inputVariable) + internal static bool IsAssignment(ILInstruction inst, out ILInstruction value) { - inputVariable = null; - ILInstruction value; + value = null; switch (inst) { case CallInstruction call: - // TODO - return false; + if (call.Method.AccessorKind != System.Reflection.MethodSemanticsAttributes.Setter) + return false; + for (int i = 0; i < call.Arguments.Count - 1; i++) { + ILInstruction arg = call.Arguments[i]; + if (arg.Flags == InstructionFlags.None) { + // OK - we accept integer literals, etc. + } else if (arg.MatchLdLoc(out var v)) { + } else { + return false; + } + } + value = call.Arguments.Last(); + return true; case StLoc stloc: value = stloc.Value; - break; + return true; case StObj stobj: // TODO return false; default: return false; } - return value.MatchLdLoc(out inputVariable); } internal override void CheckInvariant(ILPhase phase) @@ -246,8 +255,8 @@ namespace ICSharpCode.Decompiler.IL Debug.Assert(this.conversions.FinalInstruction is Nop); foreach (var inst in assignments.Instructions) { - if (!IsAssignment(inst, out var inputVariable)) - Debug.Fail("inst is not a assignment!"); + if (!(IsAssignment(inst, out var value) && value.MatchLdLoc(out var inputVariable))) + throw new InvalidOperationException("inst is not an assignment!"); Debug.Assert(patternVariables.Contains(inputVariable) || conversionVariables.Contains(inputVariable)); } Debug.Assert(this.assignments.FinalInstruction is Nop); diff --git a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs index e016e89de..01a37daa2 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs @@ -98,6 +98,86 @@ namespace ICSharpCode.Decompiler.IL } return false; } + + public ILInstruction GetCommonParent(ILInstruction other) + { + if (other == null) + throw new ArgumentNullException(nameof(other)); + + ILInstruction a = this; + ILInstruction b = other; + + int levelA = a.CountAncestors(); + int levelB = b.CountAncestors(); + + while (levelA > levelB) { + a = a.Parent; + levelA--; + } + + while (levelB > levelA) { + b = b.Parent; + levelB--; + } + + while (a != b) { + a = a.Parent; + b = b.Parent; + } + + return a; + } + + /// + /// Returns whether this appears before other in a post-order walk of the whole tree. + /// + public bool IsBefore(ILInstruction other) + { + if (other == null) + throw new ArgumentNullException(nameof(other)); + + ILInstruction a = this; + ILInstruction b = other; + + int levelA = a.CountAncestors(); + int levelB = b.CountAncestors(); + + int originalLevelA = levelA; + int originalLevelB = levelB; + + while (levelA > levelB) { + a = a.Parent; + levelA--; + } + + while (levelB > levelA) { + b = b.Parent; + levelB--; + } + + if (a == b) { + // a or b is a descendant of the other, + // whichever node has the higher level comes first in post-order walk. + return originalLevelA > originalLevelB; + } + + while (a.Parent != b.Parent) { + a = a.Parent; + b = b.Parent; + } + + // now a and b have the same parent or are both root nodes + return a.ChildIndex < b.ChildIndex; + } + + private int CountAncestors() + { + int level = 0; + for (ILInstruction ancestor = this; ancestor != null; ancestor = ancestor.Parent) { + level++; + } + return level; + } /// /// Gets the stack type of the value produced by this instruction. diff --git a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs index 3a33a5b02..d167e4adf 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DeconstructionTransform.cs @@ -18,7 +18,10 @@ using System; using System.Collections.Generic; +using System.Linq; + using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.Decompiler.Util; namespace ICSharpCode.Decompiler.IL.Transforms { @@ -62,43 +65,86 @@ namespace ICSharpCode.Decompiler.IL.Transforms try { this.context = context; this.deconstructionResultsLookup.Clear(); - int startPos = pos; - if (!MatchDeconstruction(block, ref pos, out var deconstructMethod, out var rootTestedOperand, out var deconstructionResults)) - return; - if (!MatchConversion(block, ref pos)) + + if (TransformDeconstruction(block, pos)) return; - if (!MatchAssignments(block, ref pos, out var assignments)) + if (InlineDeconstructionInitializer(block, pos)) return; - context.Step("Deconstruction", block.Instructions[startPos]); - DeconstructInstruction replacement = new DeconstructInstruction(); - IType deconstructedType; - if (deconstructMethod.IsStatic) { - deconstructedType = deconstructMethod.Parameters[0].Type; - } else { - deconstructedType = deconstructMethod.DeclaringType; - } - var rootTempVariable = context.Function.RegisterVariable(VariableKind.PatternLocal, deconstructedType); - replacement.Pattern = new MatchInstruction(rootTempVariable, deconstructMethod, rootTestedOperand) { - IsDeconstructCall = true - }; - int index = 0; - foreach (var result in deconstructionResults) { - result.Kind = VariableKind.PatternLocal; - replacement.Pattern.SubPatterns.Add(new MatchInstruction(result, new DeconstructResultInstruction(index, result.StackType, new LdLoc(rootTempVariable)))); - index++; - } - replacement.Conversions = new Block(BlockKind.DeconstructionConversions); - replacement.Assignments = new Block(BlockKind.DeconstructionAssignments); - replacement.Assignments.Instructions.AddRange(assignments); - block.Instructions[startPos] = replacement; - block.Instructions.RemoveRange(startPos + 1, pos - startPos - 1); } finally { this.context = null; this.deconstructionResultsLookup.Clear(); } } - bool MatchDeconstruction(Block block, ref int pos, out IMethod deconstructMethod, out ILInstruction testedOperand, out List deconstructionResults) + /// + /// stloc v(lhs) + /// expr(..., deconstruct { ... }, ...) + /// => + /// expr(..., deconstruct { init: stloc v(lhs) ... }, ...) + /// + bool InlineDeconstructionInitializer(Block block, int pos) + { + if (!block.Instructions[pos].MatchStLoc(out var v, out var value)) + return false; + if (!(v.IsSingleDefinition && v.LoadCount == 1)) + return false; + if (pos + 1 >= block.Instructions.Count) + return false; + var result = ILInlining.FindLoadInNext(block.Instructions[pos + 1], v, value, InliningOptions.FindDeconstruction); + if (result.Type != ILInlining.FindResultType.Deconstruction) + return false; + var deconstruction = (DeconstructInstruction)result.LoadInst; + if (!v.LoadInstructions[0].IsDescendantOf(deconstruction.Assignments)) + return false; + if (deconstruction.Init.Count > 0) { + var a = deconstruction.Init[0].Variable.LoadInstructions.Single(); + var b = v.LoadInstructions.Single(); + if (!b.IsBefore(a)) + return false; + } + context.Step("InlineDeconstructionInitializer", block.Instructions[pos]); + deconstruction.Init.Insert(0, (StLoc)block.Instructions[pos]); + block.Instructions.RemoveAt(pos); + v.Kind = VariableKind.DeconstructionInitTemporary; + return true; + } + + bool TransformDeconstruction(Block block, int pos) + { + int startPos = pos; + if (!MatchDeconstruction(block, ref pos, out var deconstructMethod, out var rootTestedOperand, out var deconstructionResults)) + return false; + if (!MatchConversion(block, ref pos)) + return false; + if (!MatchAssignments(block, ref pos, out var assignments)) + return false; + context.Step("Deconstruction", block.Instructions[startPos]); + DeconstructInstruction replacement = new DeconstructInstruction(); + IType deconstructedType; + if (deconstructMethod.IsStatic) { + deconstructedType = deconstructMethod.Parameters[0].Type; + } else { + deconstructedType = deconstructMethod.DeclaringType; + } + var rootTempVariable = context.Function.RegisterVariable(VariableKind.PatternLocal, deconstructedType); + replacement.Pattern = new MatchInstruction(rootTempVariable, deconstructMethod, rootTestedOperand) { + IsDeconstructCall = true + }; + int index = 0; + foreach (var result in deconstructionResults) { + result.Kind = VariableKind.PatternLocal; + replacement.Pattern.SubPatterns.Add(new MatchInstruction(result, new DeconstructResultInstruction(index, result.StackType, new LdLoc(rootTempVariable)))); + index++; + } + replacement.Conversions = new Block(BlockKind.DeconstructionConversions); + TransformAssignments(replacement, assignments); + block.Instructions[startPos] = replacement; + block.Instructions.RemoveRange(startPos + 1, pos - startPos - 1); + return true; + } + + bool MatchDeconstruction(Block block, ref int pos, out IMethod deconstructMethod, + out ILInstruction testedOperand, out List deconstructionResults) { testedOperand = null; deconstructMethod = null; @@ -138,22 +184,52 @@ namespace ICSharpCode.Decompiler.IL.Transforms bool MatchAssignments(Block block, ref int pos, out List assignments) { assignments = new List(); - while (MatchAssignment(block.Instructions.ElementAtOrDefault(pos))) { + int expectedIndex = 0; + while (MatchAssignment(block.Instructions.ElementAtOrDefault(pos), out var resultVariable)) { + if (!deconstructionResultsLookup.TryGetValue(resultVariable, out int index)) + return false; + if (index != expectedIndex) + return false; assignments.Add(block.Instructions[pos]); pos++; + expectedIndex++; } return assignments.Count > 0; } - bool MatchAssignment(ILInstruction inst) + bool MatchAssignment(ILInstruction inst, out ILVariable resultVariable) { - if (!DeconstructInstruction.IsAssignment(inst, out var resultVariable)) + resultVariable = null; + if (inst.MatchStLoc(out var v, out var value) + && value is Block block && block.MatchInlineAssignBlock(out var call, out var valueInst)) { + if (!DeconstructInstruction.IsAssignment(call, out _)) + return false; + if (!(v.IsSingleDefinition && v.LoadCount == 0)) + return false; + } else if (DeconstructInstruction.IsAssignment(inst, out valueInst)) { + // OK - use the assignment as is + } else { return false; - if (!deconstructionResultsLookup.ContainsKey(resultVariable)) + } + if (!valueInst.MatchLdLoc(out resultVariable)) return false; - // TODO check order of use - return true; } + + void TransformAssignments(DeconstructInstruction replacement, List assignments) + { + replacement.Assignments = new Block(BlockKind.DeconstructionAssignments); + foreach (var assignment in assignments) { + var transformed = assignment; + if (transformed.MatchStLoc(out _, out var value) + && value is Block block + && block.MatchInlineAssignBlock(out var call, out value)) { + call.Arguments[call.Arguments.Count - 1] = value; + transformed = call; + } + + replacement.Assignments.Instructions.Add(transformed); + } + } } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 5316249e2..d4963e909 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -31,6 +31,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms None = 0, Aggressive = 1, IntroduceNamedArguments = 2, + FindDeconstruction = 4, } /// @@ -543,8 +544,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// Found a load in call, but re-ordering not possible with regards to the /// other call arguments. /// Inlining is not possible, but we might convert the call to named arguments. + /// Only used with . /// NamedArgument, + /// + /// Found a deconstruction. + /// Only used with . + /// + Deconstruction, } internal readonly struct FindResult @@ -575,6 +582,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms Debug.Assert(callArg.Parent is CallInstruction); return new FindResult(FindResultType.NamedArgument, loadInst, callArg); } + + public static FindResult Deconstruction(DeconstructInstruction deconstruction) + { + return new FindResult(FindResultType.Deconstruction, deconstruction, null); + } } /// @@ -611,6 +623,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms default: return FindResult.Stop; } + } else if (options.HasFlag(InliningOptions.FindDeconstruction) && expr is DeconstructInstruction di) { + return FindResult.Deconstruction(di); } foreach (var child in expr.Children) { if (!expr.CanInlineIntoSlot(child.ChildIndex, expressionBeingMoved)) diff --git a/ILSpy/Properties/Resources.resx b/ILSpy/Properties/Resources.resx index a08d0925d..b8ccad8e0 100644 --- a/ILSpy/Properties/Resources.resx +++ b/ILSpy/Properties/Resources.resx @@ -297,6 +297,9 @@ Are you sure you want to continue? Decompile use of the 'dynamic' type + + Detect deconstruction assignments + Detect awaited using and foreach statements @@ -918,7 +921,4 @@ Do you want to continue? _Window - - Detect deconstruction assignments - \ No newline at end of file