Add documentation to InlineReturnTransform and SwitchOnStringTransform

ICSharpCode.Decompiler/IL/Transforms/InlineReturnTransform.cs

@@ -23,61 +23,73 @@ using System.Linq;
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
 	/// <summary>
-	/// This transform duplicates return blocks if they return a local variable that was assigned right before thie return.
+	/// This transform duplicates return blocks if they return a local variable that was assigned right before the return.
 	/// </summary>
 	class InlineReturnTransform : IILTransform
 	{
 		public void Run(ILFunction function, ILTransformContext context)
 		{
 			var instructionsToModify = new List<(BlockContainer, Block, Branch)>();
-			var possibleReturnVars = new Queue<(ILVariable, Block)>();
-			var tempList = new List<(BlockContainer, Block, Branch)>();
 
+			// Process all leave instructions in a leave-block, that is a block consisting solely of a leave instruction.
 			foreach (var leave in function.Descendants.OfType<Leave>()) {
-				if (!(leave.Parent is Block b && b.Instructions.Count == 1))
+				if (!(leave.Parent is Block leaveBlock && leaveBlock.Instructions.Count == 1))
 					continue;
+				// Skip, if the leave instruction has no value or the value is not a load of a local variable.
 				if (!leave.Value.MatchLdLoc(out var returnVar) || returnVar.Kind != VariableKind.Local)
 					continue;
-				possibleReturnVars.Enqueue((returnVar, b));
+				// If all instructions can be modified, add item to the global list.
+				if (CanModifyInstructions(returnVar, leaveBlock, out var list));
+					instructionsToModify.AddRange(list);
 			}
 
-			while (possibleReturnVars.Count > 0) {
-				var (returnVar, leaveBlock) = possibleReturnVars.Dequeue();
-				bool transform = true;
-				foreach (StLoc store in returnVar.StoreInstructions.OfType<StLoc>()) {
-					if (!(store.Parent is Block storeBlock)) {
-						transform = false;
-						break;
-					} 
-					if (store.ChildIndex + 2 != storeBlock.Instructions.Count) {
-						transform = false;
-						break;
-					}
-					if (!(storeBlock.Instructions[store.ChildIndex + 1] is Branch br)) {
-						transform = false;
-						break;
-					}
-					if (br.TargetBlock != leaveBlock) {
-						transform = false;
-						break;
-					}
-					var targetBlockContainer = BlockContainer.FindClosestContainer(store);
-					if (targetBlockContainer == null) {
-						transform = false;
-						break;
-					}
-					tempList.Add((targetBlockContainer, leaveBlock, br));
+			foreach (var (container, b, br) in instructionsToModify) {
+				Block block = b;
+				// if there is only one branch to this return block, move it to the matching container.
+				// otherwise duplicate the return block.
+				if (block.IncomingEdgeCount == 1) {
+					block.Remove();
+				} else {
+					block = (Block)block.Clone();
 				}
-				if (transform)
-					instructionsToModify.AddRange(tempList);
-				tempList.Clear();
+				container.Blocks.Add(block);
+				// adjust the target of the branch to the newly created block.
+				br.TargetBlock = block;
 			}
+		}
 
-			foreach (var (container, block, br) in instructionsToModify) {
-				var newBlock = (Block)block.Clone();
-				container.Blocks.Add(newBlock);
-				br.TargetBlock = newBlock;
+		/// <summary>
+		/// Determines a list of all store instructions that write to a given <paramref name="returnVar"/>.
+		/// Returns false if any of these instructions does not meet the following criteria:
+		/// - must be a stloc
+		/// - must be a direct child of a block
+		/// - must be the penultimate instruction
+		/// - must be followed by a branch instruction to <paramref name="leaveBlock"/>
+		/// - must have a BlockContainer as ancestor.
+		/// Returns true, if all instructions meet these criteria, and <paramref name="instructionsToModify"/> contains a list of 3-tuples.
+		/// Each tuple consists of the target block container, the leave block, and the branch instruction that should be modified.
+		/// </summary>
+		static bool CanModifyInstructions(ILVariable returnVar, Block leaveBlock, out List<(BlockContainer, Block, Branch)> instructionsToModify)
+		{
+			instructionsToModify = new List<(BlockContainer, Block, Branch)>();
+			foreach (var inst in returnVar.StoreInstructions) {
+				if (!(inst is StLoc store))
+					return false;
+				if (!(store.Parent is Block storeBlock))
+					return false;
+				if (store.ChildIndex + 2 != storeBlock.Instructions.Count)
+					return false;
+				if (!(storeBlock.Instructions[store.ChildIndex + 1] is Branch br))
+					return false;
+				if (br.TargetBlock != leaveBlock)
+					return false;
+				var targetBlockContainer = BlockContainer.FindClosestContainer(store);
+				if (targetBlockContainer == null)
+					return false;
+				instructionsToModify.Add((targetBlockContainer, leaveBlock, br));
 			}
+
+			return true;
 		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs

@@ -25,6 +25,9 @@ using ICSharpCode.Decompiler.Util;
 
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
+	/// <summary>
+	/// Detects switch-on-string patterns employed by the C# compiler and transforms them to an ILAst-switch-instruction.
+	/// </summary>
 	class SwitchOnStringTransform : IILTransform
 	{
 		public void Run(ILFunction function, ILTransformContext context)
@@ -78,6 +81,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			inst = null;
 			blockAfterSwitch = null;
 			// match first block: checking switch-value for null or first value (Roslyn)
+			// if (call op_Equality(ldloc switchValueVar, ldstr value)) br firstBlock
+			// -or-
+			// if (comp(ldloc switchValueVar == ldnull)) br defaultBlock
 			if (!(instructions[i].MatchIfInstruction(out var condition, out var firstBlockJump)))
 				return false;
 			if (!firstBlockJump.MatchBranch(out var firstBlock))
@@ -85,32 +91,43 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			bool isLegacy;
 			Block defaultBlock;
 			List<(string, Block)> values = new List<(string, Block)>();
+			// match null check: this is used by the old C# compiler.
 			if (condition.MatchCompEquals(out var left, out var right) && right.MatchLdNull() && left.MatchLdLoc(out var switchValueVar)) {
 				isLegacy = true;
 				defaultBlock = firstBlock;
+			// Roslyn: match call to operator ==(string, string)
 			} else if (MatchStringEqualityComparison(condition, out switchValueVar, out string value)) {
 				isLegacy = false;
 				defaultBlock = null;
 				values.Add((value, firstBlock));
 			} else return false;
+			// switchValueVar must be assigned only once.
 			if (!switchValueVar.IsSingleDefinition)
 				return false;
+			// if instruction must be followed by a branch to the next case
 			if (!(instructions.ElementAtOrDefault(i + 1) is Branch nextCaseJump))
 				return false;
+			// extract all cases and add them to the values list.
 			Block currentCaseBlock = nextCaseJump.TargetBlock;
 			Block nextCaseBlock;
 			while ((nextCaseBlock = MatchCaseBlock(currentCaseBlock, ref switchValueVar, out string value, out Block block)) != null) {
 				values.Add((value, block));
 				currentCaseBlock = nextCaseBlock;
 			}
+			// the last instruction of the last case/default block must be either
+			// a branch to the a block after the switch statement or a leave instruction.
 			var container = BlockContainer.FindClosestContainer(firstBlock);
 			if (!ExtractLastJumpFromBlock(currentCaseBlock, out var exitBlock) && !ExtractLastLeaveFromBlock(currentCaseBlock, container))
 				return false;
+			// We didn't find any cases, exit
 			if (values.Count == 0)
 				return false;
+			// All case blocks must either leave the current block container or branch to the same block after the switch statement.
 			if (!(values.All(b => ExtractLastJumpFromBlock(b.Item2, out var nextExit) && IsExitBlock(nextExit, container)) ||
 				(exitBlock == null && values.All(b => ExtractLastLeaveFromBlock(b.Item2, container)))))
 				return false;
+			// if the block after the switch has the correct number of branches, generate the switch statement
+			// and return it and the block.
 			if (currentCaseBlock.IncomingEdgeCount == (isLegacy ? 2 : 1)) {
 				var sections = new List<SwitchSection>(values.SelectWithIndex((index, b) => new SwitchSection { Labels = new LongSet(index), Body = new Branch(b.Item2) }));
 				var stringToInt = new StringToInt(new LdLoc(switchValueVar), values.SelectArray(item => item.Item1));
@@ -146,6 +163,17 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			return b == container;
 		}
 
+		/// <summary>
+		/// Each case consists of two blocks:
+		/// 1. block:
+		/// if (call op_Equality(ldloc switchVariable, ldstr value)) br caseBlock
+		/// br nextBlock
+		/// This method matches the above pattern or its inverted form:
+		/// the call to ==(string, string) is wrapped in logic.not and the branch targets are reversed.
+		/// Returns the next block that follows in the block-chain.
+		/// The <paramref name="switchVariable"/> is updated if the value gets copied to a different variable.
+		/// See comments below for more info.
+		/// </summary>
 		Block MatchCaseBlock(Block currentBlock, ref ILVariable switchVariable, out string value, out Block caseBlock)
 		{
 			value = null;
@@ -167,10 +195,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				if (!currentBlock.Instructions[1].MatchBranch(out nextBlock))
 					return null;
 			}
+			// Sometimes the switch pattern uses one variable at the beginning for null checks
+			// and another variable for the if-else-if-else-pattern.
+			// both variables must be only assigned once and be of the type: System.String.
 			if (!MatchStringEqualityComparison(condition, out var newSwitchVariable, out value))
 				return null;
 			if (!newSwitchVariable.IsSingleDefinition)
 				return null;
+			// if the used variable differs and both variables are not related, return null:
 			if (switchVariable != newSwitchVariable && !(IsInitializedBy(switchVariable, newSwitchVariable) || IsInitializedBy(newSwitchVariable, switchVariable)))
 				return null;
 			if (!newSwitchVariable.Type.IsKnownType(KnownTypeCode.String))
@@ -179,14 +211,17 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			return nextBlock;
 		}
 
-		bool IsInitializedBy(ILVariable switchVariable, ILVariable newSwitchVariable)
+		/// <summary>
+		/// Returns true if <paramref name="left"/> is only assigned once and the initialization is done by copying <paramref name="right"/>.
+		/// </summary>
+		bool IsInitializedBy(ILVariable left, ILVariable right)
 		{
-			if (!switchVariable.IsSingleDefinition)
+			if (!left.IsSingleDefinition)
 				return false;
-			var storeInst = switchVariable.StoreInstructions.OfType<StLoc>().SingleOrDefault();
+			var storeInst = left.StoreInstructions.OfType<StLoc>().SingleOrDefault();
 			if (storeInst == null)
 				return false;
-			return storeInst.Value.MatchLdLoc(newSwitchVariable);
+			return storeInst.Value.MatchLdLoc(right);
 		}
 
 		bool MatchLegacySwitchOnString(InstructionCollection<ILInstruction> instructions, int i, out SwitchInstruction inst, out Block blockAfterSwitch)