Merge pull request #2972 from ElektroKill/fix/less-return-block-duplication

ICSharpCode.Decompiler.Tests/TestCases/Pretty/ReduceNesting.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 
 namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 {
@@ -566,5 +567,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 				}
 			}
 		}
+
+		private static string ShouldNotDuplicateReturnStatementIntoTry(IDictionary<int, string> dict)
+		{
+			string value;
+			lock (dict)
+			{
+				if (!dict.TryGetValue(1, out value))
+				{
+					value = "test";
+					dict.Add(1, value);
+				}
+			}
+			return value;
+		}
 	}
 }

ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs

@@ -1,4 +1,4 @@
-// Copyright (c) 2014 Daniel Grunwald
+// Copyright (c) 2014 Daniel Grunwald
 // 
 // Permission is hereby granted, free of charge, to any person obtaining a copy of this
 // software and associated documentation files (the "Software"), to deal in the Software
@@ -137,7 +137,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 
 		void SimplifyBranchChains(ILFunction function, ILTransformContext context)
 		{
-			List<(BlockContainer, Block)> blocksToAdd = new List<(BlockContainer, Block)>();
+			List<(Block Block, BlockContainer TargetContainer)> blocksToMove = new List<(Block, BlockContainer)>();
 			HashSet<Block> visitedBlocks = new HashSet<Block>();
 			foreach (var branch in function.Descendants.OfType<Branch>())
 			{
@@ -167,7 +167,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 						context.Step("Replace branch to return with return", branch);
 						branch.ReplaceWith(targetBlock.Instructions[0].Clone());
 					}
-					else if (branch.TargetContainer != branch.Ancestors.OfType<BlockContainer>().First())
+					else if (branch.TargetContainer != branch.Ancestors.OfType<BlockContainer>().First() && targetBlock.IncomingEdgeCount == 1)
 					{
 						// We don't want to always inline the return directly, because this
 						// might force us to place the return within a loop, when it's better
@@ -175,11 +175,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 						// But we do want to move the return block into the correct try-finally scope,
 						// so that loop detection at least has the option to put it inside
 						// the loop body.
-						context.Step("Copy return block into try block", branch);
+						context.Step("Move return block into try block", branch);
-						Block blockCopy = (Block)branch.TargetBlock.Clone();
 						BlockContainer localContainer = branch.Ancestors.OfType<BlockContainer>().First();
-						blocksToAdd.Add((localContainer, blockCopy));
+						blocksToMove.Add((targetBlock, localContainer));
-						branch.TargetBlock = blockCopy;
 					}
 				}
 				else if (targetBlock.Instructions.Count == 1 && targetBlock.Instructions[0] is Leave leave && leave.Value.MatchNop())
@@ -194,9 +192,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				if (targetBlock.IncomingEdgeCount == 0)
 					targetBlock.Instructions.Clear(); // mark the block for deletion
 			}
-			foreach (var (container, block) in blocksToAdd)
+			foreach ((Block block, BlockContainer targetContainer) in blocksToMove)
 			{
-				container.Blocks.Add(block);
+				block.Remove();
+				targetContainer.Blocks.Add(block);
 			}
 		}