From 2e74e3f762e2ea9b88c9743157f4b7bf8a2540ff Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 17 Dec 2011 20:00:34 +0100 Subject: [PATCH] Fixed issue in AddCheckedBlocks that could cause variable declarations to be omitted or placed incorrectly. --- .../Ast/Transforms/AddCheckedBlocks.cs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/ICSharpCode.Decompiler/Ast/Transforms/AddCheckedBlocks.cs b/ICSharpCode.Decompiler/Ast/Transforms/AddCheckedBlocks.cs index ade17ffc01..b975d88528 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/AddCheckedBlocks.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/AddCheckedBlocks.cs @@ -45,8 +45,20 @@ sealed class CheckedUncheckedAnnotation { 1. Use minimum number of checked blocks+expressions 2. Prefer checked expressions over checked blocks 3. Make the scope of checked expressions as small as possible - 4. Make the scope of checked blocks as large as possible + 4. Open checked blocks as late as possible, and close checked blocks as late as possible (where goal 1 has the highest priority) + + Goal 4a (open checked blocks as late as possible) is necessary so that we don't move variable declarations + into checked blocks, as the variable might still be used after the checked block. + (this could cause DeclareVariables to omit the variable declaration, producing incorrect code) + Goal 4b (close checked blocks as late as possible) makes the code look nicer in this case: + checked { + int c = a + b; + int r = a + c; + return r; + } + If the checked block was closed as early as possible, the variable r would have to be declared outside + (this would work, but look badly) */ #region struct Cost @@ -257,7 +269,7 @@ Result GetResultFromBlock(BlockStatement block) Statement statement = block.Statements.FirstOrDefault(); while (true) { - // Blocks can be closed 'for free'. We use '<=' so that blocks are closed as late as possible (goal 4) + // Blocks can be closed 'for free'. We use '<=' so that blocks are closed as late as possible (goal 4b) if (costCheckedContextUncheckedBlockOpen <= costCheckedContext) { costCheckedContext = costCheckedContextUncheckedBlockOpen; nodesCheckedContext = nodesCheckedContextUncheckedBlockOpen + new InsertedBlock(uncheckedBlockStart, statement, false); @@ -268,13 +280,13 @@ Result GetResultFromBlock(BlockStatement block) } if (statement == null) break; - // Now try opening blocks. We use '<' so that blocks are opened as early as possible. (goal 4) - if (costCheckedContext + new Cost(1, 0) < costCheckedContextUncheckedBlockOpen) { + // Now try opening blocks. We use '<=' so that blocks are opened as late as possible. (goal 4a) + if (costCheckedContext + new Cost(1, 0) <= costCheckedContextUncheckedBlockOpen) { costCheckedContextUncheckedBlockOpen = costCheckedContext + new Cost(1, 0); nodesCheckedContextUncheckedBlockOpen = nodesCheckedContext; uncheckedBlockStart = statement; } - if (costUncheckedContext + new Cost(1, 0) < costUncheckedContextCheckedBlockOpen) { + if (costUncheckedContext + new Cost(1, 0) <= costUncheckedContextCheckedBlockOpen) { costUncheckedContextCheckedBlockOpen = costUncheckedContext + new Cost(1, 0); nodesUncheckedContextCheckedBlockOpen = nodesUncheckedContext; checkedBlockStart = statement;