Added support for deconstruction assignments to properties

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)
 		{

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -91,6 +91,7 @@
     <Compile Include="DisassemblerPrettyTestRunner.cs" />
     <Compile Include="ProjectDecompiler\TargetFrameworkTests.cs" />
     <Compile Include="TestAssemblyResolver.cs" />
+    <Compile Include="TestCases\Correctness\DeconstructionTests.cs" />
     <Compile Include="TestCases\Correctness\StringConcat.cs" />
     <Compile Include="TestCases\Pretty\DeconstructionTests.cs" />
     <Compile Include="TestCases\Pretty\SwitchExpressions.cs" />

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();
+			}
+		}
+	}
+}

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();
+			}
+
+
 		}
 	}
 }

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();
 				}

ICSharpCode.Decompiler/IL/ILVariable.cs

@@ -79,6 +79,10 @@ namespace ICSharpCode.Decompiler.IL
 		/// Local variable declared within a pattern match.
 		/// </summary>
 		PatternLocal,
+		/// <summary>
+		/// Temporary variable declared in a deconstruction init section.
+		/// </summary>
+		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();
 			}

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);

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;
+		}
+
+		/// <summary>
+		/// Returns whether this appears before other in a post-order walk of the whole tree.
+		/// </summary>
+		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;
+		}
 		
 		/// <summary>
 		/// Gets the stack type of the value produced by this instruction.

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<ILVariable> deconstructionResults)
+		/// <summary>
+		/// stloc v(lhs)
+		/// expr(..., deconstruct { ... }, ...)
+		/// =>
+		/// expr(..., deconstruct { init: stloc v(lhs) ... }, ...)
+		/// </summary>
+		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<ILVariable> deconstructionResults)
 		{
 			testedOperand = null;
 			deconstructMethod = null;
@@ -138,22 +184,52 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		bool MatchAssignments(Block block, ref int pos, out List<ILInstruction> assignments)
 		{
 			assignments = new List<ILInstruction>();
-			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<ILInstruction> 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);
+			}
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -31,6 +31,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		None = 0,
 		Aggressive = 1,
 		IntroduceNamedArguments = 2,
+		FindDeconstruction = 4,
 	}
 
 	/// <summary>
@@ -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 <see cref="InliningOptions.IntroduceNamedArguments"/>.
 			/// </summary>
 			NamedArgument,
+			/// <summary>
+			/// Found a deconstruction.
+			/// Only used with <see cref="InliningOptions.FindDeconstruction"/>.
+			/// </summary>
+			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);
+			}
 		}
 
 		/// <summary>
@@ -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))

ILSpy/Properties/Resources.resx

@@ -297,6 +297,9 @@ Are you sure you want to continue?</value>
   <data name="DecompilerSettings.DecompileUseOfTheDynamicType" xml:space="preserve">
     <value>Decompile use of the 'dynamic' type</value>
   </data>
+  <data name="DecompilerSettings.Deconstruction" xml:space="preserve">
+    <value>Detect deconstruction assignments</value>
+  </data>
   <data name="DecompilerSettings.DetectAsyncUsingAndForeachStatements" xml:space="preserve">
     <value>Detect awaited using and foreach statements</value>
   </data>
@@ -918,7 +921,4 @@ Do you want to continue?</value>
   <data name="_Window" xml:space="preserve">
     <value>_Window</value>
   </data>
-  <data name="DecompilerSettings.Deconstruction" xml:space="preserve">
-    <value>Detect deconstruction assignments</value>
-  </data>
 </root>