Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make return duplication in ControlFlowSimplification less aggressive #2972

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions ICSharpCode.Decompiler.Tests/TestCases/Pretty/ReduceNesting.cs
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;

namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
{
Expand Down Expand Up @@ -566,5 +567,19 @@ public void SwitchInTryInLoopContinue()
}
}
}

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;
}
}
}
@@ -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
Expand Down Expand Up @@ -137,7 +137,7 @@ void InlineVariableInReturnBlock(Block block, ILTransformContext context)

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>())
{
Expand Down Expand Up @@ -167,19 +167,17 @@ void SimplifyBranchChains(ILFunction function, ILTransformContext context)
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
// placed outside.
// 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);
Block blockCopy = (Block)branch.TargetBlock.Clone();
context.Step("Move return block into try block", branch);
BlockContainer localContainer = branch.Ancestors.OfType<BlockContainer>().First();
blocksToAdd.Add((localContainer, blockCopy));
branch.TargetBlock = blockCopy;
blocksToMove.Add((targetBlock, localContainer));
}
}
else if (targetBlock.Instructions.Count == 1 && targetBlock.Instructions[0] is Leave leave && leave.Value.MatchNop())
Expand All @@ -194,9 +192,10 @@ void SimplifyBranchChains(ILFunction function, ILTransformContext context)
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);
}
}

Expand Down