From c0a05d667214e75867f39ee98b4df34ec8a506bf Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 19 Mar 2011 22:07:07 +0100 Subject: [PATCH 01/10] Add InsertAfter/InsertBefore to AstNodeCollection. --- .../CSharp/Ast/AstNodeCollection.cs | 10 ++++++++++ .../ICSharpCode.NRefactory/TypeSystem/CecilLoader.cs | 2 +- .../TypeSystem/ITypeResolveContext.cs | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs index a8c11cb99..98beaeaef 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs @@ -193,5 +193,15 @@ namespace ICSharpCode.NRefactory.CSharp } return false; } + + public void InsertAfter(T existingItem, T newItem) + { + node.InsertChildAfter(existingItem, newItem, role); + } + + public void InsertBefore(T existingItem, T newItem) + { + node.InsertChildBefore(existingItem, newItem, role); + } } } diff --git a/NRefactory/ICSharpCode.NRefactory/TypeSystem/CecilLoader.cs b/NRefactory/ICSharpCode.NRefactory/TypeSystem/CecilLoader.cs index 74cbdb91c..26d79e4a9 100644 --- a/NRefactory/ICSharpCode.NRefactory/TypeSystem/CecilLoader.cs +++ b/NRefactory/ICSharpCode.NRefactory/TypeSystem/CecilLoader.cs @@ -35,7 +35,7 @@ namespace ICSharpCode.NRefactory.TypeSystem public bool IncludeInternalMembers { get; set; } /// - /// Gets/Sets the documentation provider that is used to retrive the XML documentation for all members. + /// Gets/Sets the documentation provider that is used to retrieve the XML documentation for all members. /// public IDocumentationProvider DocumentationProvider { get; set; } diff --git a/NRefactory/ICSharpCode.NRefactory/TypeSystem/ITypeResolveContext.cs b/NRefactory/ICSharpCode.NRefactory/TypeSystem/ITypeResolveContext.cs index 3c89b13ad..9cdfb4133 100644 --- a/NRefactory/ICSharpCode.NRefactory/TypeSystem/ITypeResolveContext.cs +++ b/NRefactory/ICSharpCode.NRefactory/TypeSystem/ITypeResolveContext.cs @@ -44,7 +44,7 @@ namespace ICSharpCode.NRefactory.TypeSystem /// Language-specific rules for how namespace names are compared /// List of classes within that namespace. /// - /// If this method is called within using (pc.Synchronize()), then the returned enumerable is valid + /// If this method is called within using (var spc = pc.Synchronize()), then the returned enumerable is valid /// only until the end of the synchronize block. /// IEnumerable GetClasses(string nameSpace, StringComparer nameComparer); @@ -53,7 +53,7 @@ namespace ICSharpCode.NRefactory.TypeSystem /// Retrieves all namespaces. /// /// - /// If this method is called within using (pc.Synchronize()), then the returned enumerable is valid + /// If this method is called within using (var spc = pc.Synchronize()), then the returned enumerable is valid /// only until the end of the synchronize block. /// IEnumerable GetNamespaces(); From 2793b233e39be10eb373f1331944d6a355be3056 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 00:54:57 +0100 Subject: [PATCH 02/10] Allow performing definite assignment analysis without providing an ITypeResolveContext. --- .../Analysis/DefiniteAssignmentTests.cs | 49 ++++++++ .../CSharp/Analysis/ControlFlow.cs | 25 ++-- .../Analysis/DefiniteAssignmentAnalysis.cs | 35 ++++-- .../CSharp/Analysis/MinimalResolveContext.cs | 116 ++++++++++++++++++ .../CSharp/OutputVisitor/OutputVisitor.cs | 2 +- .../ICSharpCode.NRefactory.csproj | 1 + 6 files changed, 209 insertions(+), 19 deletions(-) create mode 100644 NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/MinimalResolveContext.cs diff --git a/NRefactory/ICSharpCode.NRefactory.Tests/CSharp/Analysis/DefiniteAssignmentTests.cs b/NRefactory/ICSharpCode.NRefactory.Tests/CSharp/Analysis/DefiniteAssignmentTests.cs index 2e94a1c58..10373b84c 100644 --- a/NRefactory/ICSharpCode.NRefactory.Tests/CSharp/Analysis/DefiniteAssignmentTests.cs +++ b/NRefactory/ICSharpCode.NRefactory.Tests/CSharp/Analysis/DefiniteAssignmentTests.cs @@ -148,5 +148,54 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis Assert.AreEqual(DefiniteAssignmentStatus.CodeUnreachable, da.GetStatusAfter(loop.EmbeddedStatement)); Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop)); } + + [Test] + public void ForLoop() + { + ForStatement loop = new ForStatement { + Initializers = { + new ExpressionStatement( + new AssignmentExpression(new IdentifierExpression("i"), new PrimitiveExpression(0)) + ) + }, + Condition = new BinaryOperatorExpression(new IdentifierExpression("i"), BinaryOperatorType.LessThan, new PrimitiveExpression(1000)), + Iterators = { + new ExpressionStatement( + new AssignmentExpression { + Left = new IdentifierExpression("i"), + Operator = AssignmentOperatorType.Add, + Right = new IdentifierExpression("j") + } + ) + }, + EmbeddedStatement = new ExpressionStatement( + new AssignmentExpression(new IdentifierExpression("j"), new IdentifierExpression("i")) + )}; + + DefiniteAssignmentAnalysis da = new DefiniteAssignmentAnalysis(loop, CecilLoaderTests.Mscorlib); + da.Analyze("i"); + Assert.AreEqual(0, da.UnassignedVariableUses.Count); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusBefore(loop)); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusBefore(loop.Initializers.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop.Initializers.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusBeforeLoopCondition(loop)); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusBefore(loop.EmbeddedStatement)); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop.EmbeddedStatement)); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusBefore(loop.Iterators.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop.Iterators.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop)); + + da.Analyze("j"); + Assert.AreEqual(0, da.UnassignedVariableUses.Count); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusBefore(loop)); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusBefore(loop.Initializers.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusAfter(loop.Initializers.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusBeforeLoopCondition(loop)); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusBefore(loop.EmbeddedStatement)); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop.EmbeddedStatement)); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusBefore(loop.Iterators.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.DefinitelyAssigned, da.GetStatusAfter(loop.Iterators.Single())); + Assert.AreEqual(DefiniteAssignmentStatus.PotentiallyAssigned, da.GetStatusAfter(loop)); + } } } diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs index 9a2fa141d..92ffa63fa 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs @@ -146,7 +146,8 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public IList BuildControlFlowGraph(Statement statement, ITypeResolveContext context, CancellationToken cancellationToken = default(CancellationToken)) { return BuildControlFlowGraph(statement, new ResolveVisitor( - new CSharpResolver(context, cancellationToken), null, ConstantModeResolveVisitorNavigator.Skip)); + new CSharpResolver(context, cancellationToken), + null, ConstantModeResolveVisitorNavigator.Skip)); } public IList BuildControlFlowGraph(Statement statement, ResolveVisitor resolveVisitor) @@ -249,12 +250,21 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis #endregion #region Constant evaluation + /// + /// Gets/Sets whether to handle only primitive expressions as constants (no complex expressions like "a + b"). + /// + public bool EvaluateOnlyPrimitiveConstants { get; set; } + /// /// Evaluates an expression. /// /// The constant value of the expression; or null if the expression is not a constant. - ConstantResolveResult EvaluateConstant(Expression expr) + internal ConstantResolveResult EvaluateConstant(Expression expr) { + if (EvaluateOnlyPrimitiveConstants) { + if (!(expr is PrimitiveExpression || expr is NullReferenceExpression)) + return null; + } return resolveVisitor.Resolve(expr) as ConstantResolveResult; } @@ -262,11 +272,11 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// Evaluates an expression. /// /// The value of the constant boolean expression; or null if the value is not a constant boolean expression. - bool? EvaluateCondition(Expression expr) + internal bool? EvaluateCondition(Expression expr) { - ConstantResolveResult crr = EvaluateConstant(expr); - if (crr != null) - return crr.ConstantValue as bool?; + ConstantResolveResult rr = EvaluateConstant(expr); + if (rr != null) + return rr.ConstantValue as bool?; else return null; } @@ -328,7 +338,8 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis foreach (Statement stmt in statements) { if (childNode == null) { childNode = builder.CreateStartNode(stmt); - Connect(source, childNode); + if (source != null) + Connect(source, childNode); } Debug.Assert(childNode.NextStatement == stmt); childNode = stmt.AcceptVisitor(this, childNode); diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs index e37702239..c02b5ea30 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs @@ -44,10 +44,12 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// public class DefiniteAssignmentAnalysis { + readonly ControlFlowGraphBuilder cfgBuilder = new ControlFlowGraphBuilder(); readonly DefiniteAssignmentVisitor visitor = new DefiniteAssignmentVisitor(); readonly List allNodes = new List(); readonly Dictionary beginNodeDict = new Dictionary(); readonly Dictionary endNodeDict = new Dictionary(); + readonly Dictionary conditionNodeDict = new Dictionary(); readonly ResolveVisitor resolveVisitor; readonly CancellationToken cancellationToken; Dictionary nodeStatus = new Dictionary(); @@ -58,8 +60,14 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis Queue nodesWithModifiedInput = new Queue(); + public DefiniteAssignmentAnalysis(Statement rootStatement, CancellationToken cancellationToken = default(CancellationToken)) + : this(rootStatement, null, cancellationToken) + { + } + public DefiniteAssignmentAnalysis(Statement rootStatement, ITypeResolveContext context, CancellationToken cancellationToken = default(CancellationToken)) - : this(rootStatement, new ResolveVisitor(new CSharpResolver(context, cancellationToken), null, ConstantModeResolveVisitorNavigator.Skip)) + : this(rootStatement, new ResolveVisitor(new CSharpResolver(context ?? MinimalResolveContext.Instance, cancellationToken), + null, ConstantModeResolveVisitorNavigator.Skip)) { } @@ -72,16 +80,18 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis this.resolveVisitor = resolveVisitor; this.cancellationToken = resolveVisitor.CancellationToken; visitor.analysis = this; - ControlFlowGraphBuilder b = new ControlFlowGraphBuilder(); - allNodes.AddRange(b.BuildControlFlowGraph(rootStatement, resolveVisitor)); + if (resolveVisitor.TypeResolveContext is MinimalResolveContext) { + cfgBuilder.EvaluateOnlyPrimitiveConstants = true; + } + allNodes.AddRange(cfgBuilder.BuildControlFlowGraph(rootStatement, resolveVisitor)); foreach (AstNode descendant in rootStatement.Descendants) { // Anonymous methods have separate control flow graphs, but we also need to analyze those. AnonymousMethodExpression ame = descendant as AnonymousMethodExpression; if (ame != null) - allNodes.AddRange(b.BuildControlFlowGraph(ame.Body, resolveVisitor)); + allNodes.AddRange(cfgBuilder.BuildControlFlowGraph(ame.Body, resolveVisitor)); LambdaExpression lambda = descendant as LambdaExpression; if (lambda != null && lambda.Body is Statement) - allNodes.AddRange(b.BuildControlFlowGraph((Statement)lambda.Body, resolveVisitor)); + allNodes.AddRange(cfgBuilder.BuildControlFlowGraph((Statement)lambda.Body, resolveVisitor)); } // Verify that we created nodes for all statements: Debug.Assert(!rootStatement.DescendantsAndSelf.OfType().Except(allNodes.Select(n => n.NextStatement)).Any()); @@ -91,6 +101,8 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis beginNodeDict.Add(node.NextStatement, node); if (node.Type == ControlFlowNodeType.BetweenStatements || node.Type == ControlFlowNodeType.EndNode) endNodeDict.Add(node.PreviousStatement, node); + if (node.Type == ControlFlowNodeType.LoopCondition) + conditionNodeDict.Add(node.NextStatement, node); } } @@ -136,6 +148,11 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis return nodeStatus[endNodeDict[statement]]; } + public DefiniteAssignmentStatus GetStatusBeforeLoopCondition(Statement statement) + { + return nodeStatus[conditionNodeDict[statement]]; + } + /// /// Exports the CFG. This method is intended to help debugging issues related to definite assignment. /// @@ -304,7 +321,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// The constant value of the expression; or null if the expression is not a constant. ConstantResolveResult EvaluateConstant(Expression expr) { - return resolveVisitor.Resolve(expr) as ConstantResolveResult; + return cfgBuilder.EvaluateConstant(expr); } /// @@ -313,11 +330,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// The value of the constant boolean expression; or null if the value is not a constant boolean expression. bool? EvaluateCondition(Expression expr) { - ConstantResolveResult crr = EvaluateConstant(expr); - if (crr != null) - return crr.ConstantValue as bool?; - else - return null; + return cfgBuilder.EvaluateCondition(expr); } static DefiniteAssignmentStatus CleanSpecialValues(DefiniteAssignmentStatus status) diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/MinimalResolveContext.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/MinimalResolveContext.cs new file mode 100644 index 000000000..d9909d0d4 --- /dev/null +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/MinimalResolveContext.cs @@ -0,0 +1,116 @@ +// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt) +// This code is distributed under MIT X11 license (for details please see \doc\license.txt) + +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Linq; + +using ICSharpCode.NRefactory.TypeSystem; +using ICSharpCode.NRefactory.TypeSystem.Implementation; + +namespace ICSharpCode.NRefactory.CSharp.Analysis +{ + /// + /// Resolve context represents the minimal mscorlib required for evaluating constants. + /// + sealed class MinimalResolveContext : IProjectContent, ISynchronizedTypeResolveContext + { + static readonly Lazy instance = new Lazy(() => new MinimalResolveContext()); + + public static MinimalResolveContext Instance { + get { return instance.Value; } + } + + readonly ReadOnlyCollection namespaces = Array.AsReadOnly(new string[] { "System" }); + readonly IAttribute[] assemblyAttributes = new IAttribute[0]; + readonly ITypeDefinition systemObject, systemValueType; + readonly ReadOnlyCollection types; + + private MinimalResolveContext() + { + List types = new List(); + types.Add(systemObject = new DefaultTypeDefinition(this, "System", "Object")); + types.Add(systemValueType = new DefaultTypeDefinition(this, "System", "ValueType") { BaseTypes = { systemObject } }); + types.Add(CreateStruct("System", "Boolean")); + types.Add(CreateStruct("System", "SByte")); + types.Add(CreateStruct("System", "Byte")); + types.Add(CreateStruct("System", "Int16")); + types.Add(CreateStruct("System", "UInt16")); + types.Add(CreateStruct("System", "Int32")); + types.Add(CreateStruct("System", "UInt32")); + types.Add(CreateStruct("System", "Int64")); + types.Add(CreateStruct("System", "UInt64")); + types.Add(CreateStruct("System", "Single")); + types.Add(CreateStruct("System", "Double")); + types.Add(CreateStruct("System", "Decimal")); + types.Add(new DefaultTypeDefinition(this, "System", "String") { BaseTypes = { systemObject } }); + foreach (ITypeDefinition type in types) + type.Freeze(); + this.types = types.AsReadOnly(); + } + + ITypeDefinition CreateStruct(string nameSpace, string name) + { + return new DefaultTypeDefinition(this, nameSpace, name) { + ClassType = ClassType.Struct, + BaseTypes = { systemValueType } + }; + } + + public ITypeDefinition GetClass(string nameSpace, string name, int typeParameterCount, StringComparer nameComparer) + { + foreach (ITypeDefinition type in types) { + if (nameComparer.Equals(type.Name, name) && nameComparer.Equals(type.Namespace, nameSpace) && type.TypeParameterCount == typeParameterCount) + return type; + } + return null; + } + + public IEnumerable GetClasses() + { + return types; + } + + public IEnumerable GetClasses(string nameSpace, StringComparer nameComparer) + { + return types.Where(t => nameComparer.Equals(t.Namespace, nameSpace)); + } + + public IEnumerable GetNamespaces() + { + return namespaces; + } + + public string GetNamespace(string nameSpace, StringComparer nameComparer) + { + foreach (string ns in namespaces) { + if (nameComparer.Equals(ns, nameSpace)) + return ns; + } + return null; + } + + public IList AssemblyAttributes { + get { return assemblyAttributes; } + } + + ICSharpCode.NRefactory.Utils.CacheManager ITypeResolveContext.CacheManager { + get { + // We don't support caching + return null; + } + } + + ISynchronizedTypeResolveContext ITypeResolveContext.Synchronize() + { + // This class is immutable + return this; + } + + void IDisposable.Dispose() + { + // exit from Synchronize() block + } + } +} diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs index da843f39c..947028699 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs @@ -428,7 +428,7 @@ namespace ICSharpCode.NRefactory.CSharp if (block != null) VisitBlockStatement(block, null); else - throw new NotImplementedException(); + embeddedStatement.AcceptVisitor(this, null); } void WriteMethodBody(BlockStatement body) diff --git a/NRefactory/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj b/NRefactory/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj index 6a57c5315..e8fdec0b5 100644 --- a/NRefactory/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj +++ b/NRefactory/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj @@ -63,6 +63,7 @@ + From a2fb74bee68ef1ce55e9de31e62683c54e5ece54 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 01:56:26 +0100 Subject: [PATCH 03/10] Order the control flow nodes lexically, and allow restricting definite assignment analysis to a specific lexical range. --- .../CSharp/Analysis/ControlFlow.cs | 36 +++-- .../Analysis/DefiniteAssignmentAnalysis.cs | 151 +++++++++++++----- 2 files changed, 129 insertions(+), 58 deletions(-) diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs index 92ffa63fa..1660e99b1 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/ControlFlow.cs @@ -222,14 +222,15 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis return node; } - ControlFlowNode CreateSpecialNode(Statement statement, ControlFlowNodeType type) + ControlFlowNode CreateSpecialNode(Statement statement, ControlFlowNodeType type, bool addToNodeList = true) { ControlFlowNode node = CreateNode(null, statement, type); - nodes.Add(node); + if (addToNodeList) + nodes.Add(node); return node; } - ControlFlowNode CreateEndNode(Statement statement) + ControlFlowNode CreateEndNode(Statement statement, bool addToNodeList = true) { Statement nextStatement; if (statement == rootStatement) { @@ -244,7 +245,8 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis } ControlFlowNodeType type = nextStatement != null ? ControlFlowNodeType.BetweenStatements : ControlFlowNodeType.EndNode; ControlFlowNode node = CreateNode(statement, nextStatement, type); - nodes.Add(node); + if (addToNodeList) + nodes.Add(node); return node; } #endregion @@ -259,7 +261,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// Evaluates an expression. /// /// The constant value of the expression; or null if the expression is not a constant. - internal ConstantResolveResult EvaluateConstant(Expression expr) + ConstantResolveResult EvaluateConstant(Expression expr) { if (EvaluateOnlyPrimitiveConstants) { if (!(expr is PrimitiveExpression || expr is NullReferenceExpression)) @@ -272,7 +274,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// Evaluates an expression. /// /// The value of the constant boolean expression; or null if the value is not a constant boolean expression. - internal bool? EvaluateCondition(Expression expr) + bool? EvaluateCondition(Expression expr) { ConstantResolveResult rr = EvaluateConstant(expr); if (rr != null) @@ -418,7 +420,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis int gotoCaseOrDefaultInOuterScope = gotoCaseOrDefault.Count; - ControlFlowNode end = builder.CreateEndNode(switchStatement); + ControlFlowNode end = builder.CreateEndNode(switchStatement, addToNodeList: false); breakTargets.Push(end); foreach (SwitchSection section in switchStatement.SwitchSections) { if (constant == null || section == sectionMatchedByConstant) { @@ -439,6 +441,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis throw new NotImplementedException(); } + builder.nodes.Add(end); return end; } @@ -457,7 +460,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public override ControlFlowNode VisitWhileStatement(WhileStatement whileStatement, ControlFlowNode data) { // while (cond) { embeddedStmt; } - ControlFlowNode end = builder.CreateEndNode(whileStatement); + ControlFlowNode end = builder.CreateEndNode(whileStatement, addToNodeList: false); ControlFlowNode conditionNode = builder.CreateSpecialNode(whileStatement, ControlFlowNodeType.LoopCondition); breakTargets.Push(end); continueTargets.Push(conditionNode); @@ -475,14 +478,15 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis breakTargets.Pop(); continueTargets.Pop(); + builder.nodes.Add(end); return end; } public override ControlFlowNode VisitDoWhileStatement(DoWhileStatement doWhileStatement, ControlFlowNode data) { // do { embeddedStmt; } while(cond); - ControlFlowNode end = builder.CreateEndNode(doWhileStatement); - ControlFlowNode conditionNode = builder.CreateSpecialNode(doWhileStatement, ControlFlowNodeType.LoopCondition); + ControlFlowNode end = builder.CreateEndNode(doWhileStatement, addToNodeList: false); + ControlFlowNode conditionNode = builder.CreateSpecialNode(doWhileStatement, ControlFlowNodeType.LoopCondition, addToNodeList: false); breakTargets.Push(end); continueTargets.Push(conditionNode); @@ -499,6 +503,8 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis breakTargets.Pop(); continueTargets.Pop(); + builder.nodes.Add(conditionNode); + builder.nodes.Add(end); return end; } @@ -506,7 +512,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis { data = HandleStatementList(forStatement.Initializers, data); // for (initializers ; cond; iterators) { embeddedStmt; } - ControlFlowNode end = builder.CreateEndNode(forStatement); + ControlFlowNode end = builder.CreateEndNode(forStatement, addToNodeList: false); ControlFlowNode conditionNode = builder.CreateSpecialNode(forStatement, ControlFlowNodeType.LoopCondition); Connect(data, conditionNode); @@ -536,6 +542,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis if (cond != true) Connect(conditionNode, end, ControlFlowEdgeType.ConditionFalse); + builder.nodes.Add(end); return end; } @@ -552,7 +559,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public override ControlFlowNode VisitForeachStatement(ForeachStatement foreachStatement, ControlFlowNode data) { // foreach (...) { embeddedStmt } - ControlFlowNode end = builder.CreateEndNode(foreachStatement); + ControlFlowNode end = builder.CreateEndNode(foreachStatement, addToNodeList: false); ControlFlowNode conditionNode = builder.CreateSpecialNode(foreachStatement, ControlFlowNodeType.LoopCondition); Connect(data, conditionNode); @@ -566,7 +573,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis continueTargets.Pop(); Connect(conditionNode, end); - + builder.nodes.Add(end); return end; } @@ -602,7 +609,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public override ControlFlowNode VisitTryCatchStatement(TryCatchStatement tryCatchStatement, ControlFlowNode data) { - ControlFlowNode end = builder.CreateEndNode(tryCatchStatement); + ControlFlowNode end = builder.CreateEndNode(tryCatchStatement, addToNodeList: false); var edge = Connect(HandleEmbeddedStatement(tryCatchStatement.TryBlock, data), end); if (!tryCatchStatement.FinallyBlock.IsNull) edge.AddJumpOutOfTryFinally(tryCatchStatement); @@ -616,6 +623,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis // Consumers of the CFG will have to special-case try-finally. HandleEmbeddedStatement(tryCatchStatement.FinallyBlock, data); } + builder.nodes.Add(end); return end; } diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs index c02b5ea30..fb723aedc 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs @@ -44,21 +44,40 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// public class DefiniteAssignmentAnalysis { - readonly ControlFlowGraphBuilder cfgBuilder = new ControlFlowGraphBuilder(); + sealed class DefiniteAssignmentNode : ControlFlowNode + { + public int Index; + public DefiniteAssignmentStatus NodeStatus; + + public DefiniteAssignmentNode(Statement previousStatement, Statement nextStatement, ControlFlowNodeType type) + : base(previousStatement, nextStatement, type) + { + } + } + + sealed class DerivedControlFlowGraphBuilder : ControlFlowGraphBuilder + { + protected override ControlFlowNode CreateNode(Statement previousStatement, Statement nextStatement, ControlFlowNodeType type) + { + return new DefiniteAssignmentNode(previousStatement, nextStatement, type); + } + } + + readonly DerivedControlFlowGraphBuilder cfgBuilder = new DerivedControlFlowGraphBuilder(); readonly DefiniteAssignmentVisitor visitor = new DefiniteAssignmentVisitor(); - readonly List allNodes = new List(); - readonly Dictionary beginNodeDict = new Dictionary(); - readonly Dictionary endNodeDict = new Dictionary(); - readonly Dictionary conditionNodeDict = new Dictionary(); + readonly List allNodes = new List(); + readonly Dictionary beginNodeDict = new Dictionary(); + readonly Dictionary endNodeDict = new Dictionary(); + readonly Dictionary conditionNodeDict = new Dictionary(); readonly ResolveVisitor resolveVisitor; readonly CancellationToken cancellationToken; - Dictionary nodeStatus = new Dictionary(); Dictionary edgeStatus = new Dictionary(); string variableName; List unassignedVariableUses = new List(); + int analyzedRangeStart, analyzedRangeEnd; - Queue nodesWithModifiedInput = new Queue(); + Queue nodesWithModifiedInput = new Queue(); public DefiniteAssignmentAnalysis(Statement rootStatement, CancellationToken cancellationToken = default(CancellationToken)) : this(rootStatement, null, cancellationToken) @@ -83,20 +102,18 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis if (resolveVisitor.TypeResolveContext is MinimalResolveContext) { cfgBuilder.EvaluateOnlyPrimitiveConstants = true; } - allNodes.AddRange(cfgBuilder.BuildControlFlowGraph(rootStatement, resolveVisitor)); - foreach (AstNode descendant in rootStatement.Descendants) { - // Anonymous methods have separate control flow graphs, but we also need to analyze those. - AnonymousMethodExpression ame = descendant as AnonymousMethodExpression; - if (ame != null) - allNodes.AddRange(cfgBuilder.BuildControlFlowGraph(ame.Body, resolveVisitor)); - LambdaExpression lambda = descendant as LambdaExpression; - if (lambda != null && lambda.Body is Statement) - allNodes.AddRange(cfgBuilder.BuildControlFlowGraph((Statement)lambda.Body, resolveVisitor)); - } - // Verify that we created nodes for all statements: - Debug.Assert(!rootStatement.DescendantsAndSelf.OfType().Except(allNodes.Select(n => n.NextStatement)).Any()); - // Now register the nodes in the dictionaries: - foreach (ControlFlowNode node in allNodes) { + allNodes.AddRange(cfgBuilder.BuildControlFlowGraph(rootStatement, resolveVisitor).Cast()); + for (int i = 0; i < allNodes.Count; i++) { + DefiniteAssignmentNode node = allNodes[i]; + node.Index = i; // assign numbers to the nodes + if (node.Type == ControlFlowNodeType.StartNode || node.Type == ControlFlowNodeType.BetweenStatements) { + // Anonymous methods have separate control flow graphs, but we also need to analyze those. + // Iterate backwards so that anonymous methods are inserted in the correct order + for (AstNode child = node.NextStatement.LastChild; child != null; child = child.PrevSibling) { + InsertAnonymousMethods(i + 1, child); + } + } + // Now register the node in the dictionaries: if (node.Type == ControlFlowNodeType.StartNode || node.Type == ControlFlowNodeType.BetweenStatements) beginNodeDict.Add(node.NextStatement, node); if (node.Type == ControlFlowNodeType.BetweenStatements || node.Type == ControlFlowNodeType.EndNode) @@ -104,6 +121,32 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis if (node.Type == ControlFlowNodeType.LoopCondition) conditionNodeDict.Add(node.NextStatement, node); } + // Verify that we created nodes for all statements: + Debug.Assert(!rootStatement.DescendantsAndSelf.OfType().Except(allNodes.Select(n => n.NextStatement)).Any()); + this.analyzedRangeStart = 0; + this.analyzedRangeEnd = allNodes.Count - 1; + } + + void InsertAnonymousMethods(int insertPos, AstNode node) + { + // Ignore any statements, as those have their own ControlFlowNode and get handled separately + if (node is Statement) + return; + AnonymousMethodExpression ame = node as AnonymousMethodExpression; + if (ame != null) { + allNodes.InsertRange(insertPos, cfgBuilder.BuildControlFlowGraph(ame.Body, resolveVisitor).Cast()); + return; + } + LambdaExpression lambda = node as LambdaExpression; + if (lambda != null && lambda.Body is Statement) { + allNodes.InsertRange(insertPos, cfgBuilder.BuildControlFlowGraph((Statement)lambda.Body, resolveVisitor).Cast()); + return; + } + // Descend into child expressions + // Iterate backwards so that anonymous methods are inserted in the correct order + for (AstNode child = node.LastChild; child != null; child = child.PrevSibling) { + InsertAnonymousMethods(insertPos, child); + } } /// @@ -115,21 +158,37 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis } } + /// + /// Sets the range of statements to be analyzed. + /// This method can be used to restrict the analysis to only a part of the method. + /// Only the control flow paths that are fully contained within the selected part will be analyzed. + /// + /// Both 'start' and 'end' are inclusive. + public void SetAnalyzedRange(Statement start, Statement end) + { + int startIndex = beginNodeDict[start].Index; + int endIndex = endNodeDict[end].Index; + if (startIndex > endIndex) + throw new ArgumentException("The start statement must be lexically preceding the end statement"); + this.analyzedRangeStart = startIndex; + this.analyzedRangeEnd = endIndex; + } + public void Analyze(string variable, DefiniteAssignmentStatus initialStatus = DefiniteAssignmentStatus.PotentiallyAssigned) { this.variableName = variable; // Reset the status: unassignedVariableUses.Clear(); - foreach (ControlFlowNode node in allNodes) { - nodeStatus[node] = DefiniteAssignmentStatus.CodeUnreachable; + foreach (DefiniteAssignmentNode node in allNodes) { + node.NodeStatus = DefiniteAssignmentStatus.CodeUnreachable; foreach (ControlFlowEdge edge in node.Outgoing) edgeStatus[edge] = DefiniteAssignmentStatus.CodeUnreachable; } - ChangeNodeStatus(allNodes[0], initialStatus); + ChangeNodeStatus(allNodes[analyzedRangeStart], initialStatus); // Iterate as long as the input status of some nodes is changing: while (nodesWithModifiedInput.Count > 0) { - ControlFlowNode node = nodesWithModifiedInput.Dequeue(); + DefiniteAssignmentNode node = nodesWithModifiedInput.Dequeue(); DefiniteAssignmentStatus inputStatus = DefiniteAssignmentStatus.CodeUnreachable; foreach (ControlFlowEdge edge in node.Incoming) { inputStatus = MergeStatus(inputStatus, edgeStatus[edge]); @@ -140,17 +199,17 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public DefiniteAssignmentStatus GetStatusBefore(Statement statement) { - return nodeStatus[beginNodeDict[statement]]; + return beginNodeDict[statement].NodeStatus; } public DefiniteAssignmentStatus GetStatusAfter(Statement statement) { - return nodeStatus[endNodeDict[statement]]; + return endNodeDict[statement].NodeStatus; } public DefiniteAssignmentStatus GetStatusBeforeLoopCondition(Statement statement) { - return nodeStatus[conditionNodeDict[statement]]; + return conditionNodeDict[statement].NodeStatus; } /// @@ -161,7 +220,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis GraphVizGraph g = new GraphVizGraph(); g.Title = "DefiniteAssignment - " + variableName; for (int i = 0; i < allNodes.Count; i++) { - string name = nodeStatus[allNodes[i]].ToString() + Environment.NewLine; + string name = "#" + i + " = " + allNodes[i].NodeStatus.ToString() + Environment.NewLine; switch (allNodes[i].Type) { case ControlFlowNodeType.StartNode: case ControlFlowNodeType.BetweenStatements: @@ -179,7 +238,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis } g.AddNode(new GraphVizNode(i) { label = name }); foreach (ControlFlowEdge edge in allNodes[i].Outgoing) { - GraphVizEdge ge = new GraphVizEdge(i, allNodes.IndexOf(edge.To)); + GraphVizEdge ge = new GraphVizEdge(i, ((DefiniteAssignmentNode)edge.To).Index); if (edgeStatus.Count > 0) ge.label = edgeStatus[edge].ToString(); if (edge.IsLeavingTryFinally) @@ -218,11 +277,11 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis return DefiniteAssignmentStatus.PotentiallyAssigned; } - void ChangeNodeStatus(ControlFlowNode node, DefiniteAssignmentStatus inputStatus) + void ChangeNodeStatus(DefiniteAssignmentNode node, DefiniteAssignmentStatus inputStatus) { - if (nodeStatus[node] == inputStatus) + if (node.NodeStatus == inputStatus) return; - nodeStatus[node] = inputStatus; + node.NodeStatus = inputStatus; DefiniteAssignmentStatus outputStatus; switch (node.Type) { case ControlFlowNodeType.StartNode: @@ -312,7 +371,9 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis throw new InvalidOperationException("Invalid value for DefiniteAssignmentStatus"); } edgeStatus[edge] = newStatus; - nodesWithModifiedInput.Enqueue(edge.To); + DefiniteAssignmentNode targetNode = (DefiniteAssignmentNode)edge.To; + if (analyzedRangeStart <= targetNode.Index && targetNode.Index <= analyzedRangeEnd) + nodesWithModifiedInput.Enqueue(targetNode); } /// @@ -321,7 +382,11 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// The constant value of the expression; or null if the expression is not a constant. ConstantResolveResult EvaluateConstant(Expression expr) { - return cfgBuilder.EvaluateConstant(expr); + if (resolveVisitor.TypeResolveContext is MinimalResolveContext) { + if (!(expr is PrimitiveExpression || expr is NullReferenceExpression)) + return null; + } + return resolveVisitor.Resolve(expr) as ConstantResolveResult; } /// @@ -330,7 +395,11 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// The value of the constant boolean expression; or null if the value is not a constant boolean expression. bool? EvaluateCondition(Expression expr) { - return cfgBuilder.EvaluateCondition(expr); + ConstantResolveResult rr = EvaluateConstant(expr); + if (rr != null) + return rr.ConstantValue as bool?; + else + return null; } static DefiniteAssignmentStatus CleanSpecialValues(DefiniteAssignmentStatus status) @@ -633,10 +702,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public override DefiniteAssignmentStatus VisitAnonymousMethodExpression(AnonymousMethodExpression anonymousMethodExpression, DefiniteAssignmentStatus data) { BlockStatement body = anonymousMethodExpression.Body; - foreach (ControlFlowNode node in analysis.allNodes) { - if (node.NextStatement == body) - analysis.ChangeNodeStatus(node, data); - } + analysis.ChangeNodeStatus(analysis.beginNodeDict[body], data); return data; } @@ -644,10 +710,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis { Statement body = lambdaExpression.Body as Statement; if (body != null) { - foreach (ControlFlowNode node in analysis.allNodes) { - if (node.NextStatement == body) - analysis.ChangeNodeStatus(node, data); - } + analysis.ChangeNodeStatus(analysis.beginNodeDict[body], data); } else { lambdaExpression.Body.AcceptVisitor(this, data); } From f537fafdeb5c3c9c8291ca699ec9aa31c8fbe04d Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 02:34:55 +0100 Subject: [PATCH 04/10] Add NextStatement/PreviousStatement properties. --- .../CSharp/Ast/Statements/Statement.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs index ef01d54b3..41cbe67f5 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs @@ -38,6 +38,38 @@ namespace ICSharpCode.NRefactory.CSharp } #endregion + /// + /// Gets the previous statement within the current block. + /// This is usually equivalent to , but will skip any non-statements (e.g. comments) + /// + public Statement PreviousStatement { + get { + AstNode node = this; + while ((node = node.PrevSibling) != null) { + Statement stmt = node as Statement; + if (stmt != null) + return stmt; + } + return null; + } + } + + /// + /// Gets the next statement within the current block. + /// This is usually equivalent to , but will skip any non-statements (e.g. comments) + /// + public Statement NextStatement { + get { + AstNode node = this; + while ((node = node.NextSibling) != null) { + Statement stmt = node as Statement; + if (stmt != null) + return stmt; + } + return null; + } + } + public new Statement Clone() { return (Statement)base.Clone(); From 1331869851e729d9a08529a5ff21d12c764b877d Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 14:42:38 +0100 Subject: [PATCH 05/10] Fixed definite assignment analysis bug. --- .../CSharp/Analysis/DefiniteAssignmentAnalysis.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs index fb723aedc..0c9b4ac28 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs @@ -559,11 +559,14 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis { IdentifierExpression ident = left as IdentifierExpression; if (ident != null && ident.Identifier == analysis.variableName) { - right.AcceptVisitor(this, initialStatus); + // right==null is special case when handling 'out' expressions + if (right != null) + right.AcceptVisitor(this, initialStatus); return DefiniteAssignmentStatus.DefinitelyAssigned; } else { DefiniteAssignmentStatus status = left.AcceptVisitor(this, initialStatus); - status = right.AcceptVisitor(this, CleanSpecialValues(status)); + if (right != null) + status = right.AcceptVisitor(this, CleanSpecialValues(status)); return CleanSpecialValues(status); } } From c7bbdcd0cb81c182318f188a1ef4e077ed0f403f Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 02:59:59 +0100 Subject: [PATCH 06/10] Improved variable placement. The variable placement step now happens much later in the decompiler pipeline, which simplifies some AST transforms. --- .../Ast/AstMethodBodyBuilder.cs | 8 +- .../Ast/DeclareVariableInSmallestScope.cs | 91 ------- .../Ast/Transforms/DeclareVariables.cs | 255 ++++++++++++++++++ .../Ast/Transforms/DelegateConstruction.cs | 27 +- .../Transforms/IntroduceUsingDeclarations.cs | 2 +- .../Transforms/PatternStatementTransform.cs | 190 +++++-------- .../Ast/Transforms/TransformationPipeline.cs | 1 + .../ICSharpCode.Decompiler.csproj | 2 +- 8 files changed, 345 insertions(+), 231 deletions(-) delete mode 100644 ICSharpCode.Decompiler/Ast/DeclareVariableInSmallestScope.cs create mode 100644 ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs diff --git a/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs b/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs index b6c626e6a..6bd62aad8 100644 --- a/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs +++ b/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs @@ -22,7 +22,6 @@ namespace ICSharpCode.Decompiler.Ast TypeSystem typeSystem; DecompilerContext context; HashSet localVariablesToDefine = new HashSet(); // local variables that are missing a definition - HashSet implicitlyDefinedVariables = new HashSet(); // local variables that are implicitly defined (e.g. catch handler) /// /// Creates the body for the method definition. @@ -89,8 +88,9 @@ namespace ICSharpCode.Decompiler.Ast context.CancellationToken.ThrowIfCancellationRequested(); Ast.BlockStatement astBlock = TransformBlock(ilMethod); CommentStatement.ReplaceAll(astBlock); // convert CommentStatements to Comments - foreach (ILVariable v in localVariablesToDefine.Except(implicitlyDefinedVariables)) { - DeclareVariableInSmallestScope.DeclareVariable(astBlock, AstBuilder.ConvertType(v.Type), v.Name); + foreach (ILVariable v in localVariablesToDefine) { + var newVarDecl = new VariableDeclarationStatement(AstBuilder.ConvertType(v.Type), v.Name); + astBlock.Statements.InsertAfter(null, newVarDecl); } return astBlock; @@ -158,8 +158,6 @@ namespace ICSharpCode.Decompiler.Ast var tryCatchStmt = new Ast.TryCatchStatement(); tryCatchStmt.TryBlock = TransformBlock(tryCatchNode.TryBlock); foreach (var catchClause in tryCatchNode.CatchBlocks) { - if (catchClause.ExceptionVariable != null) - implicitlyDefinedVariables.Add(catchClause.ExceptionVariable); tryCatchStmt.CatchClauses.Add( new Ast.CatchClause { Type = AstBuilder.ConvertType(catchClause.ExceptionType), diff --git a/ICSharpCode.Decompiler/Ast/DeclareVariableInSmallestScope.cs b/ICSharpCode.Decompiler/Ast/DeclareVariableInSmallestScope.cs deleted file mode 100644 index f609c2414..000000000 --- a/ICSharpCode.Decompiler/Ast/DeclareVariableInSmallestScope.cs +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt) -// This code is distributed under MIT X11 license (for details please see \doc\license.txt) - -using System; -using System.Linq; -using ICSharpCode.NRefactory.CSharp; -using ICSharpCode.NRefactory.CSharp.PatternMatching; - -namespace ICSharpCode.Decompiler.Ast -{ - /// - /// Helper class for declaring variables. - /// - public static class DeclareVariableInSmallestScope - { - static readonly ExpressionStatement assignmentPattern = new ExpressionStatement( - new AssignmentExpression( - new NamedNode("ident", new IdentifierExpression()), - new AnyNode("init") - )); - - /// - /// Declares a variable in the smallest required scope. - /// - /// The root of the subtree being searched for the best insertion position - /// The type of the new variable - /// The name of the new variable - /// Whether the variable is allowed to be placed inside a loop - public static VariableDeclarationStatement DeclareVariable(AstNode node, AstType type, string name, bool allowPassIntoLoops = true) - { - VariableDeclarationStatement result = null; - AstNode pos = FindInsertPos(node, name, allowPassIntoLoops); - if (pos != null) { - Match m = assignmentPattern.Match(pos); - if (m != null && m.Get("ident").Single().Identifier == name) { - result = new VariableDeclarationStatement(type, name, m.Get("init").Single().Detach()); - result.Variables.Single().CopyAnnotationsFrom(((ExpressionStatement)pos).Expression); - result.CopyAnnotationsFrom(pos); - pos.ReplaceWith(result); - } else { - result = new VariableDeclarationStatement(type, name); - pos.Parent.InsertChildBefore(pos, result, BlockStatement.StatementRole); - } - } - return result; - } - - static AstNode FindInsertPos(AstNode node, string name, bool allowPassIntoLoops) - { - AstNode pos = null; - AstNode withinPos = null; - while (node != null) { - IdentifierExpression ident = node as IdentifierExpression; - if (ident != null && ident.Identifier == name && ident.TypeArguments.Count == 0) - return node; - - FixedStatement fixedStatement = node as FixedStatement; - if (fixedStatement != null) { - foreach (VariableInitializer v in fixedStatement.Variables) { - if (v.Name == name) - return null; // no need to introduce the variable here - } - } - - AstNode withinCurrent = FindInsertPos(node.FirstChild, name, allowPassIntoLoops); - if (withinCurrent != null) { - if (pos == null) { - pos = node; - withinPos = withinCurrent; - } else { - return pos; - } - } - node = node.NextSibling; - } - if (withinPos != null && withinPos.Role == BlockStatement.StatementRole && AllowPassInto(pos, allowPassIntoLoops)) - return withinPos; - else - return pos; - } - - static bool AllowPassInto(AstNode node, bool allowPassIntoLoops) - { - if (node is AnonymousMethodExpression || node is LambdaExpression) - return false; - if (node is ForStatement || node is ForeachStatement || node is DoWhileStatement || node is WhileStatement) - return allowPassIntoLoops; - return true; - } - } -} diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs new file mode 100644 index 000000000..0d85de650 --- /dev/null +++ b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs @@ -0,0 +1,255 @@ +// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt) +// This code is distributed under MIT X11 license (for details please see \doc\license.txt) + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using ICSharpCode.NRefactory.CSharp; +using ICSharpCode.NRefactory.CSharp.Analysis; + +namespace ICSharpCode.Decompiler.Ast.Transforms +{ + /// + /// Moves variable declarations to improved positions. + /// + public class DeclareVariables : IAstTransform + { + sealed class DeclaredVariableAnnotation { + public readonly ExpressionStatement OriginalAssignmentStatement; + + public DeclaredVariableAnnotation(ExpressionStatement originalAssignmentStatement) + { + this.OriginalAssignmentStatement = originalAssignmentStatement; + } + } + static readonly DeclaredVariableAnnotation declaredVariableAnnotation = new DeclaredVariableAnnotation(null); + + readonly CancellationToken cancellationToken; + + public DeclareVariables(DecompilerContext context) + { + this.cancellationToken = context.CancellationToken; + } + + public void Run(AstNode node) + { + Run(node, null); + } + + void Run(AstNode node, DefiniteAssignmentAnalysis daa) + { + BlockStatement block = node as BlockStatement; + if (block != null) { + var variables = block.Statements.TakeWhile(stmt => stmt is VariableDeclarationStatement + && stmt.Annotation() == null) + .Cast().ToList(); + if (variables.Count > 0) { + // remove old variable declarations: + foreach (VariableDeclarationStatement varDecl in variables) { + Debug.Assert(varDecl.Variables.Single().Initializer.IsNull); + varDecl.Remove(); + } + if (daa == null) { + // If possible, reuse the DefiniteAssignmentAnalysis that was created for the parent block + daa = new DefiniteAssignmentAnalysis(block, cancellationToken); + } + foreach (VariableDeclarationStatement varDecl in variables) { + string variableName = varDecl.Variables.Single().Name; + bool allowPassIntoLoops = varDecl.Variables.Single().Annotation() == null; + DeclareVariableInBlock(daa, block, varDecl.Type, variableName, allowPassIntoLoops); + } + } + } + for (AstNode child = node.FirstChild; child != null; child = child.NextSibling) { + Run(child, daa); + } + } + + void DeclareVariableInBlock(DefiniteAssignmentAnalysis daa, BlockStatement block, AstType type, string variableName, bool allowPassIntoLoops) + { + // declarationPoint: The point where the variable would be declared, if we decide to declare it in this block + Statement declarationPoint = null; + // Check whether we can move down the variable into the sub-blocks + bool ok = true; + foreach (Statement stmt in block.Statements) { + if (UsesVariable(stmt, variableName)) { + if (declarationPoint == null) + declarationPoint = stmt; + if (!CanMoveVariableUseIntoSubBlock(stmt, variableName, allowPassIntoLoops)) { + // If it's not possible to move the variable use into a nested block, + // we need to declare the variable in this block + ok = false; + break; + } + // If we can move the variable into the sub-block, we need to ensure that the remaining code + // does not use the value that was assigend by the first sub-block + Statement nextStatement = stmt.NextStatement; + // The next statement might be a variable declaration that we inserted, and thus does not exist + // in the definite assignment graph. Thus we need to look up the corresponding instruction + // prior to the introduction of the VariableDeclarationStatement. + while (nextStatement is VariableDeclarationStatement) { + DeclaredVariableAnnotation annotation = nextStatement.Annotation(); + if (annotation == null) + break; + if (annotation.OriginalAssignmentStatement != null) { + nextStatement = annotation.OriginalAssignmentStatement; + break; + } + nextStatement = nextStatement.NextStatement; + } + if (nextStatement != null) { + // Analyze the range from the next statement to the end of the block + daa.SetAnalyzedRange(nextStatement, block); + daa.Analyze(variableName); + if (daa.UnassignedVariableUses.Count > 0) { + ok = false; + break; + } + } + } + } + if (declarationPoint == null) { + // The variable isn't used at all + return; + } + if (ok) { + // Declare the variable within the sub-blocks + foreach (Statement stmt in block.Statements) { + foreach (BlockStatement subBlock in stmt.Children.OfType()) { + DeclareVariableInBlock(daa, subBlock, type, variableName, allowPassIntoLoops); + } + } + } else { + // Try converting an assignment expression into a VariableDeclarationStatement + ExpressionStatement es = declarationPoint as ExpressionStatement; + if (es != null) { + AssignmentExpression ae = es.Expression as AssignmentExpression; + if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { + IdentifierExpression ident = ae.Left as IdentifierExpression; + if (ident != null && ident.Identifier == variableName) { + // convert the declarationPoint into a VariableDeclarationStatement + declarationPoint.ReplaceWith( + new VariableDeclarationStatement { + Type = (AstType)type.Clone(), + Variables = { + new VariableInitializer(variableName, ae.Right.Detach()).CopyAnnotationsFrom(ae) + } + }.CopyAnnotationsFrom(es).WithAnnotation(new DeclaredVariableAnnotation(es))); + return; + } + } + } + // Declare the variable in front of declarationPoint + block.Statements.InsertBefore( + declarationPoint, + new VariableDeclarationStatement((AstType)type.Clone(), variableName) + .WithAnnotation(declaredVariableAnnotation)); + } + } + + bool CanMoveVariableUseIntoSubBlock(Statement stmt, string variableName, bool allowPassIntoLoops) + { + if (!allowPassIntoLoops && (stmt is ForStatement || stmt is ForeachStatement || stmt is DoWhileStatement || stmt is WhileStatement)) + return false; + ForStatement forStatement = stmt as ForStatement; + if (forStatement != null && forStatement.Initializers.Count == 1) { + // for-statement is special case: we can move variable declarations into the initializer + ExpressionStatement es = forStatement.Initializers.Single() as ExpressionStatement; + if (es != null) { + AssignmentExpression ae = es.Expression as AssignmentExpression; + if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { + IdentifierExpression ident = ae.Left as IdentifierExpression; + if (ident != null && ident.Identifier == variableName) { + return !UsesVariable(ae.Right, variableName); + } + } + } + } + // We can move the variable into a sub-block only if the variable is used in only that sub-block + for (AstNode child = stmt.FirstChild; child != null; child = child.NextSibling) { + if (!(child is BlockStatement) && UsesVariable(child, variableName)) + return false; + } + return true; + } + + bool UsesVariable(AstNode node, string variableName) + { + IdentifierExpression ie = node as IdentifierExpression; + if (ie != null && ie.Identifier == variableName) + return true; + + FixedStatement fixedStatement = node as FixedStatement; + if (fixedStatement != null) { + foreach (VariableInitializer v in fixedStatement.Variables) { + if (v.Name == variableName) + return false; // no need to introduce the variable here + } + } + + ForeachStatement foreachStatement = node as ForeachStatement; + if (foreachStatement != null) { + if (foreachStatement.VariableName == variableName) + return false; // no need to introduce the variable here + } + + for (AstNode child = node.FirstChild; child != null; child = child.NextSibling) { + if (UsesVariable(child, variableName)) + return true; + } + return false; + } + + #region FindInsertPos + static AstNode FindInsertPos(AstNode node, string name, bool allowPassIntoLoops) + { + AstNode pos = null; + AstNode withinPos = null; + while (node != null) { + IdentifierExpression ident = node as IdentifierExpression; + if (ident != null && ident.Identifier == name && ident.TypeArguments.Count == 0) + return node; + + FixedStatement fixedStatement = node as FixedStatement; + if (fixedStatement != null) { + foreach (VariableInitializer v in fixedStatement.Variables) { + if (v.Name == name) + return null; // no need to introduce the variable here + } + } + ForeachStatement foreachStatement = node as ForeachStatement; + if (foreachStatement != null) { + if (foreachStatement.VariableName == name) + return null; // no need to introduce the variable here + } + + AstNode withinCurrent = FindInsertPos(node.FirstChild, name, allowPassIntoLoops); + if (withinCurrent != null) { + if (pos == null) { + pos = node; + withinPos = withinCurrent; + } else { + return pos; + } + } + node = node.NextSibling; + } + if (withinPos != null && withinPos.Role == BlockStatement.StatementRole && AllowPassInto(pos, allowPassIntoLoops)) + return withinPos; + else + return pos; + } + + static bool AllowPassInto(AstNode node, bool allowPassIntoLoops) + { + if (node is AnonymousMethodExpression || node is LambdaExpression) + return false; + + return allowPassIntoLoops; + return true; + } + #endregion + } +} diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs index 48a2ddf6c..b619230a9 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs @@ -179,23 +179,33 @@ namespace ICSharpCode.Decompiler.Ast.Transforms return true; } + static readonly ExpressionStatement displayClassAssignmentPattern = + new ExpressionStatement(new AssignmentExpression( + new NamedNode("variable", new IdentifierExpression()), + new ObjectCreateExpression { Type = new AnyNode("type") } + )); + public override object VisitBlockStatement(BlockStatement blockStatement, object data) { base.VisitBlockStatement(blockStatement, data); - foreach (VariableDeclarationStatement stmt in blockStatement.Statements.OfType().ToArray()) { - if (stmt.Variables.Count() != 1) + foreach (ExpressionStatement stmt in blockStatement.Statements.OfType().ToArray()) { + Match displayClassAssignmentMatch = displayClassAssignmentPattern.Match(stmt); + if (displayClassAssignmentMatch == null) + continue; + + ILVariable variable = displayClassAssignmentMatch.Get("variable").Single().Annotation(); + if (variable == null) continue; - var variable = stmt.Variables.Single(); - TypeDefinition type = stmt.Type.Annotation().ResolveWithinSameModule(); + TypeDefinition type = variable.Type.ResolveWithinSameModule(); if (!IsPotentialClosure(context, type)) continue; - ObjectCreateExpression oce = variable.Initializer as ObjectCreateExpression; - if (oce == null || oce.Type.Annotation().ResolveWithinSameModule() != type || oce.Arguments.Any() || !oce.Initializer.IsNull) + if (displayClassAssignmentMatch.Get("type").Single().Annotation().ResolveWithinSameModule() != type) continue; + // Looks like we found a display class creation. Now let's verify that the variable is used only for field accesses: bool ok = true; foreach (var identExpr in blockStatement.Descendants.OfType()) { - if (identExpr.Identifier == variable.Name) { + if (identExpr.Identifier == variable.Name && identExpr != displayClassAssignmentMatch.Get("variable").Single()) { if (!(identExpr.Parent is MemberReferenceExpression && identExpr.Parent.Annotation() != null)) ok = false; } @@ -267,9 +277,10 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } // Now insert the variable declarations (we can do this after the replacements only so that the scope detection works): foreach (var tuple in variablesToDeclare) { - var newVarDecl = DeclareVariableInSmallestScope.DeclareVariable(blockStatement, tuple.Item1, tuple.Item2, allowPassIntoLoops: false); + var newVarDecl = new VariableDeclarationStatement(tuple.Item1, tuple.Item2); if (newVarDecl != null) newVarDecl.Variables.Single().AddAnnotation(new CapturedVariableAnnotation()); + blockStatement.Statements.InsertAfter(null, newVarDecl); } } return null; diff --git a/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs b/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs index 0b0a40564..9b0c3d713 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs @@ -25,7 +25,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms public void Run(AstNode compilationUnit) { // Don't show using when decompiling a single method or nested types: - if (context.CurrentMethod != null || (context.CurrentType != null && context.CurrentType.IsNested)) + if (context.CurrentType != null) return; // First determine all the namespaces that need to be imported: diff --git a/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs index e2d6536d3..6cdc1f5da 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using ICSharpCode.Decompiler.ILAst; using ICSharpCode.NRefactory.CSharp; using ICSharpCode.NRefactory.CSharp.PatternMatching; using Mono.Cecil; @@ -37,23 +38,23 @@ namespace ICSharpCode.Decompiler.Ast.Transforms return node; } - public override AstNode VisitVariableDeclarationStatement(VariableDeclarationStatement variableDeclarationStatement, object data) + public override AstNode VisitExpressionStatement(ExpressionStatement expressionStatement, object data) { AstNode result; if (context.Settings.UsingStatement) { - result = TransformUsings(variableDeclarationStatement); + result = TransformUsings(expressionStatement); if (result != null) return result; } - result = TransformFor(variableDeclarationStatement); + result = TransformFor(expressionStatement); if (result != null) return result; if (context.Settings.LockStatement) { - result = TransformLock(variableDeclarationStatement); + result = TransformLock(expressionStatement); if (result != null) return result; } - return base.VisitVariableDeclarationStatement(variableDeclarationStatement, data); + return base.VisitExpressionStatement(expressionStatement, data); } public override AstNode VisitUsingStatement(UsingStatement usingStatement, object data) @@ -117,27 +118,11 @@ namespace ICSharpCode.Decompiler.Ast.Transforms /// /// $type $variable = $initializer; /// - static readonly AstNode variableDeclPattern = new VariableDeclarationStatement { - Type = new AnyNode("type"), - Variables = { - new NamedNode( - "variable", - new VariableInitializer { - Initializer = new AnyNode("initializer") - } - ) - } - }; - - /// - /// Variable declaration without initializer. - /// - static readonly AstNode simpleVariableDefinition = new VariableDeclarationStatement { - Type = new AnyNode(), - Variables = { - new VariableInitializer() // any name but no initializer - } - }; + static readonly AstNode variableAssignPattern = new ExpressionStatement( + new AssignmentExpression( + new NamedNode("variable", new IdentifierExpression()), + new AnyNode("initializer") + )); #region using static readonly AstNode usingTryCatchPattern = new TryCatchStatement { @@ -163,20 +148,18 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } }; - public UsingStatement TransformUsings(VariableDeclarationStatement node) + public UsingStatement TransformUsings(ExpressionStatement node) { - Match m1 = variableDeclPattern.Match(node); + Match m1 = variableAssignPattern.Match(node); if (m1 == null) return null; AstNode tryCatch = node.NextSibling; - while (simpleVariableDefinition.Match(tryCatch) != null) - tryCatch = tryCatch.NextSibling; Match m2 = usingTryCatchPattern.Match(tryCatch); if (m2 == null) return null; - if (m1.Get("variable").Single().Name == m2.Get("ident").Single().Identifier) { + if (m1.Get("variable").Single().Identifier == m2.Get("ident").Single().Identifier) { if (m2.Has("valueType")) { // if there's no if(x!=null), then it must be a value type - TypeReference tr = m1.Get("type").Single().Annotation(); - if (tr == null || !tr.IsValueType) + ILVariable v = m1.Get("variable").Single().Annotation(); + if (v == null || v.Type == null || !v.Type.IsValueType) return null; } BlockStatement body = m2.Get("body").Single(); @@ -184,6 +167,8 @@ namespace ICSharpCode.Decompiler.Ast.Transforms usingStatement.ResourceAcquisition = node.Detach(); usingStatement.EmbeddedStatement = body.Detach(); tryCatch.ReplaceWith(usingStatement); + // TODO: Move the variable declaration into the resource acquisition, if possible + // This is necessary for the foreach-pattern to work on the result of the using-pattern return usingStatement; } return null; @@ -203,55 +188,22 @@ namespace ICSharpCode.Decompiler.Ast.Transforms ) } }, - EmbeddedStatement = new Choice { - // There are two forms of the foreach statement: - // one where the item variable is declared inside the loop, - // and one where it is declared outside of the loop. - // In the former case, we can apply the foreach pattern only if the variable wasn't captured. - { "itemVariableInsideLoop", - new BlockStatement { - new WhileStatement { - Condition = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Invoke("MoveNext"), - EmbeddedStatement = new BlockStatement { - new VariableDeclarationStatement { - Type = new AnyNode("itemType"), - Variables = { - new NamedNode( - "itemVariable", - new VariableInitializer { - Initializer = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Member("Current") - } - ) - } - }, - new Repeat(new AnyNode("statement")).ToStatement() - } - } - } - }, - { "itemVariableOutsideLoop", - new BlockStatement { - new VariableDeclarationStatement { - Type = new AnyNode("itemType"), - Variables = { - new NamedNode("itemVariable", new VariableInitializer()) - } + EmbeddedStatement = new BlockStatement { + new Repeat( + new NamedNode("variableDeclaration", new VariableDeclarationStatement { Type = new AnyNode(), Variables = { new AnyNode() } }) + ).ToStatement(), + new WhileStatement { + Condition = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Invoke("MoveNext"), + EmbeddedStatement = new BlockStatement { + new AssignmentExpression { + Left = new NamedNode("itemVariable", new IdentifierExpression()), + Operator = AssignmentOperatorType.Assign, + Right = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Member("Current") }, - new WhileStatement { - Condition = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Invoke("MoveNext"), - EmbeddedStatement = new BlockStatement { - new AssignmentExpression { - Left = new IdentifierExpressionBackreference("itemVariable"), - Operator = AssignmentOperatorType.Assign, - Right = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Member("Current") - }, - new Repeat(new AnyNode("statement")).ToStatement() - } - } + new Repeat(new AnyNode("statement")).ToStatement() } } - }.ToStatement() - }; + }}; public ForeachStatement TransformForeach(UsingStatement node) { @@ -259,14 +211,18 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (m == null) return null; VariableInitializer enumeratorVar = m.Get("enumeratorVariable").Single(); - VariableInitializer itemVar = m.Get("itemVariable").Single(); - if (m.Has("itemVariableInsideLoop") && itemVar.Annotation() != null) { - // cannot move captured variables out of loops + ILVariable itemVar = m.Get("itemVariable").Single().Annotation(); + if (itemVar == null) return null; - } + return null; // TODO +// if (m.Has("itemVariableInsideLoop") && itemVar.Annotation() != null) { +// // cannot move captured variables out of loops +// return null; +// } BlockStatement newBody = new BlockStatement(); foreach (Statement stmt in m.Get("statement")) newBody.Add(stmt.Detach()); + ForeachStatement foreachStatement = new ForeachStatement { VariableType = m.Get("itemType").Single().Detach(), VariableName = itemVar.Name, @@ -299,17 +255,15 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } }}; - public ForStatement TransformFor(VariableDeclarationStatement node) + public ForStatement TransformFor(ExpressionStatement node) { - Match m1 = variableDeclPattern.Match(node); + Match m1 = variableAssignPattern.Match(node); if (m1 == null) return null; AstNode next = node.NextSibling; - while (simpleVariableDefinition.Match(next) != null) - next = next.NextSibling; Match m2 = forPattern.Match(next); if (m2 == null) return null; // ensure the variable in the for pattern is the same as in the declaration - if (m1.Get("variable").Single().Name != m2.Get("ident").Single().Identifier) + if (m1.Get("variable").Single().Identifier != m2.Get("ident").Single().Identifier) return null; WhileStatement loop = (WhileStatement)next; node.Remove(); @@ -374,16 +328,11 @@ namespace ICSharpCode.Decompiler.Ast.Transforms #endregion #region lock - static readonly AstNode lockFlagInitPattern = new VariableDeclarationStatement { - Type = new PrimitiveType("bool"), - Variables = { - new NamedNode( - "variable", - new VariableInitializer { - Initializer = new PrimitiveExpression(false) - } - ) - }}; + static readonly AstNode lockFlagInitPattern = new ExpressionStatement( + new AssignmentExpression( + new NamedNode("variable", new IdentifierExpression()), + new PrimitiveExpression(false) + )); static readonly AstNode lockTryCatchPattern = new TryCatchStatement { TryBlock = new BlockStatement { @@ -404,16 +353,14 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } }}; - public LockStatement TransformLock(VariableDeclarationStatement node) + public LockStatement TransformLock(ExpressionStatement node) { Match m1 = lockFlagInitPattern.Match(node); if (m1 == null) return null; AstNode tryCatch = node.NextSibling; - while (simpleVariableDefinition.Match(tryCatch) != null) - tryCatch = tryCatch.NextSibling; Match m2 = lockTryCatchPattern.Match(tryCatch); if (m2 == null) return null; - if (m1.Get("variable").Single().Name == m2.Get("flag").Single().Identifier) { + if (m1.Get("variable").Single().Identifier == m2.Get("flag").Single().Identifier) { Expression enter = m2.Get("enter").Single(); IdentifierExpression exit = m2.Get("exit").Single(); if (exit.Match(enter) == null) { @@ -630,32 +577,25 @@ namespace ICSharpCode.Decompiler.Ast.Transforms #region Automatic Events static readonly Accessor automaticEventPatternV4 = new Accessor { Body = new BlockStatement { - new VariableDeclarationStatement { - Type = new AnyNode("type"), - Variables = { - new NamedNode( - "var1", new VariableInitializer { - Initializer = new NamedNode("field", new MemberReferenceExpression { Target = new ThisReferenceExpression() }) - })} - }, - new VariableDeclarationStatement { - Type = new Backreference("type"), - Variables = { new NamedNode("var2", new VariableInitializer()) } + new VariableDeclarationStatement { Type = new AnyNode("type"), Variables = { new AnyNode() } }, + new VariableDeclarationStatement { Type = new Backreference("type"), Variables = { new AnyNode() } }, + new VariableDeclarationStatement { Type = new Backreference("type"), Variables = { new AnyNode() } }, + new AssignmentExpression { + Left = new NamedNode("var1", new IdentifierExpression()), + Operator = AssignmentOperatorType.Assign, + Right = new NamedNode("field", new MemberReferenceExpression { Target = new ThisReferenceExpression() }) }, new DoWhileStatement { EmbeddedStatement = new BlockStatement { - new AssignmentExpression(new IdentifierExpressionBackreference("var2"), new IdentifierExpressionBackreference("var1")), - new VariableDeclarationStatement { - Type = new Backreference("type"), - Variables = { - new NamedNode( - "var3", new VariableInitializer { - Initializer = new AnyNode("delegateCombine").ToExpression().Invoke( - new IdentifierExpressionBackreference("var2"), - new IdentifierExpression("value") - ).CastTo(new Backreference("type")) - }) - }}, + new AssignmentExpression(new NamedNode("var2", new IdentifierExpression()), new IdentifierExpressionBackreference("var1")), + new AssignmentExpression { + Left = new NamedNode("var3", new IdentifierExpression()), + Operator = AssignmentOperatorType.Assign, + Right = new AnyNode("delegateCombine").ToExpression().Invoke( + new IdentifierExpressionBackreference("var2"), + new IdentifierExpression("value") + ).CastTo(new Backreference("type")) + }, new AssignmentExpression { Left = new IdentifierExpressionBackreference("var1"), Right = new TypePattern(typeof(System.Threading.Interlocked)).ToType().Invoke( diff --git a/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs b/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs index eff00c6ed..57a5ada6c 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs @@ -23,6 +23,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms new ConvertConstructorCallIntoInitializer(), new ReplaceMethodCallsWithOperators(), new IntroduceUnsafeModifier(), + new DeclareVariables(context), new IntroduceUsingDeclarations(context) }; } diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 945b4d705..b7c2151d6 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -53,13 +53,13 @@ - + From 8e3f62ba1475ec908f85dc625a2086c4378fc906 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 13:37:57 +0100 Subject: [PATCH 07/10] Fix issues with 'DeclareVariables' and adjust 'foreach' pattern to the new variable handling. --- .../Ast/NRefactoryExtensions.cs | 11 + .../Ast/Transforms/DeclareVariables.cs | 221 ++++++++++-------- .../Transforms/IntroduceUsingDeclarations.cs | 2 +- .../Transforms/PatternStatementTransform.cs | 104 +++++++-- .../Ast/Transforms/TransformationPipeline.cs | 4 +- 5 files changed, 230 insertions(+), 112 deletions(-) diff --git a/ICSharpCode.Decompiler/Ast/NRefactoryExtensions.cs b/ICSharpCode.Decompiler/Ast/NRefactoryExtensions.cs index 8781b6ac6..5589fd500 100644 --- a/ICSharpCode.Decompiler/Ast/NRefactoryExtensions.cs +++ b/ICSharpCode.Decompiler/Ast/NRefactoryExtensions.cs @@ -3,6 +3,7 @@ using System; using ICSharpCode.NRefactory.CSharp; +using ICSharpCode.NRefactory.CSharp.PatternMatching; namespace ICSharpCode.Decompiler.Ast { @@ -29,6 +30,16 @@ namespace ICSharpCode.Decompiler.Ast return node; } + public static Expression WithName(this Expression node, string patternGroupName) + { + return new NamedNode(patternGroupName, node); + } + + public static Statement WithName(this Statement node, string patternGroupName) + { + return new NamedNode(patternGroupName, node); + } + public static void AddNamedArgument(this NRefactory.CSharp.Attribute attribute, string name, Expression argument) { attribute.Arguments.Add(new AssignmentExpression(new IdentifierExpression(name), argument)); diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs index 0d85de650..fe2cba560 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs @@ -72,7 +72,103 @@ namespace ICSharpCode.Decompiler.Ast.Transforms // declarationPoint: The point where the variable would be declared, if we decide to declare it in this block Statement declarationPoint = null; // Check whether we can move down the variable into the sub-blocks - bool ok = true; + bool canMoveVariableIntoSubBlocks = FindDeclarationPoint(daa, variableName, allowPassIntoLoops, block, out declarationPoint); + if (declarationPoint == null) { + // The variable isn't used at all + return; + } + if (canMoveVariableIntoSubBlocks) { + // Declare the variable within the sub-blocks + foreach (Statement stmt in block.Statements) { + ForStatement forStmt = stmt as ForStatement; + if (forStmt != null && forStmt.Initializers.Count == 1) { + // handle the special case of moving a variable into the for initializer + if (TryConvertAssignmentExpressionIntoVariableDeclaration(forStmt.Initializers.Single(), type, variableName)) + continue; + } + UsingStatement usingStmt = stmt as UsingStatement; + if (usingStmt != null && usingStmt.ResourceAcquisition is AssignmentExpression) { + // handle the special case of moving a variable into a using statement + if (TryConvertAssignmentExpressionIntoVariableDeclaration((Expression)usingStmt.ResourceAcquisition, type, variableName)) + continue; + } + foreach (BlockStatement subBlock in stmt.Children.OfType()) { + DeclareVariableInBlock(daa, subBlock, type, variableName, allowPassIntoLoops); + } + } + } else { + // Try converting an assignment expression into a VariableDeclarationStatement + if (!TryConvertAssignmentExpressionIntoVariableDeclaration(declarationPoint, type, variableName)) { + // Declare the variable in front of declarationPoint + block.Statements.InsertBefore( + declarationPoint, + new VariableDeclarationStatement((AstType)type.Clone(), variableName) + .WithAnnotation(declaredVariableAnnotation)); + } + } + } + + bool TryConvertAssignmentExpressionIntoVariableDeclaration(Statement declarationPoint, AstType type, string variableName) + { + // convert the declarationPoint into a VariableDeclarationStatement + ExpressionStatement es = declarationPoint as ExpressionStatement; + if (es != null) { + AssignmentExpression ae = es.Expression as AssignmentExpression; + if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { + IdentifierExpression ident = ae.Left as IdentifierExpression; + if (ident != null && ident.Identifier == variableName) { + declarationPoint.ReplaceWith(new VariableDeclarationStatement { + Type = (AstType)type.Clone(), + Variables = { new VariableInitializer(variableName, ae.Right.Detach()).CopyAnnotationsFrom(ae) } + }.CopyAnnotationsFrom(es).WithAnnotation(new DeclaredVariableAnnotation(es))); + return true; + } + } + } + return false; + } + + bool TryConvertAssignmentExpressionIntoVariableDeclaration(Expression expression, AstType type, string variableName) + { + AssignmentExpression ae = expression as AssignmentExpression; + if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { + IdentifierExpression ident = ae.Left as IdentifierExpression; + if (ident != null && ident.Identifier == variableName) { + expression.ReplaceWith(new VariableDeclarationStatement { + Type = (AstType)type.Clone(), + Variables = { new VariableInitializer(variableName, ae.Right.Detach()).CopyAnnotationsFrom(ae) } + }.WithAnnotation(declaredVariableAnnotation)); + return true; + } + } + return false; + } + + /// + /// Finds the declaration point for the variable within the specified block. + /// + /// + /// Definite assignment analysis, must be prepared for 'block' or one of its parents. + /// + /// The variable to declare + /// The block in which the variable should be declared + /// + /// Output parameter: the first statement within 'block' where the variable needs to be declared. + /// + /// + /// Returns whether it is possible to move the variable declaration into sub-blocks. + /// + public static bool FindDeclarationPoint(DefiniteAssignmentAnalysis daa, VariableDeclarationStatement varDecl, BlockStatement block, out Statement declarationPoint) + { + string variableName = varDecl.Variables.Single().Name; + bool allowPassIntoLoops = varDecl.Variables.Single().Annotation() == null; + return FindDeclarationPoint(daa, variableName, allowPassIntoLoops, block, out declarationPoint); + } + + static bool FindDeclarationPoint(DefiniteAssignmentAnalysis daa, string variableName, bool allowPassIntoLoops, BlockStatement block, out Statement declarationPoint) + { + // declarationPoint: The point where the variable would be declared, if we decide to declare it in this block + declarationPoint = null; foreach (Statement stmt in block.Statements) { if (UsesVariable(stmt, variableName)) { if (declarationPoint == null) @@ -80,8 +176,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (!CanMoveVariableUseIntoSubBlock(stmt, variableName, allowPassIntoLoops)) { // If it's not possible to move the variable use into a nested block, // we need to declare the variable in this block - ok = false; - break; + return false; } // If we can move the variable into the sub-block, we need to ensure that the remaining code // does not use the value that was assigend by the first sub-block @@ -104,55 +199,19 @@ namespace ICSharpCode.Decompiler.Ast.Transforms daa.SetAnalyzedRange(nextStatement, block); daa.Analyze(variableName); if (daa.UnassignedVariableUses.Count > 0) { - ok = false; - break; + return false; } } } } - if (declarationPoint == null) { - // The variable isn't used at all - return; - } - if (ok) { - // Declare the variable within the sub-blocks - foreach (Statement stmt in block.Statements) { - foreach (BlockStatement subBlock in stmt.Children.OfType()) { - DeclareVariableInBlock(daa, subBlock, type, variableName, allowPassIntoLoops); - } - } - } else { - // Try converting an assignment expression into a VariableDeclarationStatement - ExpressionStatement es = declarationPoint as ExpressionStatement; - if (es != null) { - AssignmentExpression ae = es.Expression as AssignmentExpression; - if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { - IdentifierExpression ident = ae.Left as IdentifierExpression; - if (ident != null && ident.Identifier == variableName) { - // convert the declarationPoint into a VariableDeclarationStatement - declarationPoint.ReplaceWith( - new VariableDeclarationStatement { - Type = (AstType)type.Clone(), - Variables = { - new VariableInitializer(variableName, ae.Right.Detach()).CopyAnnotationsFrom(ae) - } - }.CopyAnnotationsFrom(es).WithAnnotation(new DeclaredVariableAnnotation(es))); - return; - } - } - } - // Declare the variable in front of declarationPoint - block.Statements.InsertBefore( - declarationPoint, - new VariableDeclarationStatement((AstType)type.Clone(), variableName) - .WithAnnotation(declaredVariableAnnotation)); - } + return true; } - bool CanMoveVariableUseIntoSubBlock(Statement stmt, string variableName, bool allowPassIntoLoops) + static bool CanMoveVariableUseIntoSubBlock(Statement stmt, string variableName, bool allowPassIntoLoops) { if (!allowPassIntoLoops && (stmt is ForStatement || stmt is ForeachStatement || stmt is DoWhileStatement || stmt is WhileStatement)) return false; + ForStatement forStatement = stmt as ForStatement; if (forStatement != null && forStatement.Initializers.Count == 1) { // for-statement is special case: we can move variable declarations into the initializer @@ -167,6 +226,19 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } } } + + UsingStatement usingStatement = stmt as UsingStatement; + if (usingStatement != null) { + // using-statement is special case: we can move variable declarations into the initializer + AssignmentExpression ae = usingStatement.ResourceAcquisition as AssignmentExpression; + if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { + IdentifierExpression ident = ae.Left as IdentifierExpression; + if (ident != null && ident.Identifier == variableName) { + return !UsesVariable(ae.Right, variableName); + } + } + } + // We can move the variable into a sub-block only if the variable is used in only that sub-block for (AstNode child = stmt.FirstChild; child != null; child = child.NextSibling) { if (!(child is BlockStatement) && UsesVariable(child, variableName)) @@ -175,7 +247,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms return true; } - bool UsesVariable(AstNode node, string variableName) + static bool UsesVariable(AstNode node, string variableName) { IdentifierExpression ie = node as IdentifierExpression; if (ie != null && ie.Identifier == variableName) @@ -195,61 +267,22 @@ namespace ICSharpCode.Decompiler.Ast.Transforms return false; // no need to introduce the variable here } + UsingStatement usingStatement = node as UsingStatement; + if (usingStatement != null) { + VariableDeclarationStatement varDecl = usingStatement.ResourceAcquisition as VariableDeclarationStatement; + if (varDecl != null) { + foreach (VariableInitializer v in varDecl.Variables) { + if (v.Name == variableName) + return false; // no need to introduce the variable here + } + } + } + for (AstNode child = node.FirstChild; child != null; child = child.NextSibling) { if (UsesVariable(child, variableName)) return true; } return false; } - - #region FindInsertPos - static AstNode FindInsertPos(AstNode node, string name, bool allowPassIntoLoops) - { - AstNode pos = null; - AstNode withinPos = null; - while (node != null) { - IdentifierExpression ident = node as IdentifierExpression; - if (ident != null && ident.Identifier == name && ident.TypeArguments.Count == 0) - return node; - - FixedStatement fixedStatement = node as FixedStatement; - if (fixedStatement != null) { - foreach (VariableInitializer v in fixedStatement.Variables) { - if (v.Name == name) - return null; // no need to introduce the variable here - } - } - ForeachStatement foreachStatement = node as ForeachStatement; - if (foreachStatement != null) { - if (foreachStatement.VariableName == name) - return null; // no need to introduce the variable here - } - - AstNode withinCurrent = FindInsertPos(node.FirstChild, name, allowPassIntoLoops); - if (withinCurrent != null) { - if (pos == null) { - pos = node; - withinPos = withinCurrent; - } else { - return pos; - } - } - node = node.NextSibling; - } - if (withinPos != null && withinPos.Role == BlockStatement.StatementRole && AllowPassInto(pos, allowPassIntoLoops)) - return withinPos; - else - return pos; - } - - static bool AllowPassInto(AstNode node, bool allowPassIntoLoops) - { - if (node is AnonymousMethodExpression || node is LambdaExpression) - return false; - - return allowPassIntoLoops; - return true; - } - #endregion } } diff --git a/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs b/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs index 9b0c3d713..be3998c6c 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/IntroduceUsingDeclarations.cs @@ -25,7 +25,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms public void Run(AstNode compilationUnit) { // Don't show using when decompiling a single method or nested types: - if (context.CurrentType != null) + if (context.CurrentMethod != null || (context.CurrentType != null && context.CurrentType.DeclaringType != null)) return; // First determine all the namespaces that need to be imported: diff --git a/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs index 6cdc1f5da..71c56e6a6 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Linq; using ICSharpCode.Decompiler.ILAst; using ICSharpCode.NRefactory.CSharp; +using ICSharpCode.NRefactory.CSharp.Analysis; using ICSharpCode.NRefactory.CSharp.PatternMatching; using Mono.Cecil; @@ -155,24 +156,75 @@ namespace ICSharpCode.Decompiler.Ast.Transforms AstNode tryCatch = node.NextSibling; Match m2 = usingTryCatchPattern.Match(tryCatch); if (m2 == null) return null; - if (m1.Get("variable").Single().Identifier == m2.Get("ident").Single().Identifier) { + string variableName = m1.Get("variable").Single().Identifier; + if (variableName == m2.Get("ident").Single().Identifier) { if (m2.Has("valueType")) { // if there's no if(x!=null), then it must be a value type ILVariable v = m1.Get("variable").Single().Annotation(); if (v == null || v.Type == null || !v.Type.IsValueType) return null; } + node.Remove(); BlockStatement body = m2.Get("body").Single(); UsingStatement usingStatement = new UsingStatement(); - usingStatement.ResourceAcquisition = node.Detach(); + usingStatement.ResourceAcquisition = node.Expression.Detach(); usingStatement.EmbeddedStatement = body.Detach(); tryCatch.ReplaceWith(usingStatement); - // TODO: Move the variable declaration into the resource acquisition, if possible + // Move the variable declaration into the resource acquisition, if possible // This is necessary for the foreach-pattern to work on the result of the using-pattern + VariableDeclarationStatement varDecl = FindVariableDeclaration(usingStatement, variableName); + if (varDecl != null && varDecl.Parent is BlockStatement) { + Statement declarationPoint; + if (CanMoveVariableDeclarationIntoStatement(varDecl, usingStatement, out declarationPoint)) { + Debug.Assert(declarationPoint == usingStatement); + // Moving the variable into the UsingStatement is allowed: + usingStatement.ResourceAcquisition = new VariableDeclarationStatement { + Type = (AstType)varDecl.Type.Clone(), + Variables = { + new VariableInitializer { + Name = variableName, + Initializer = m1.Get("initializer").Single().Detach() + }.CopyAnnotationsFrom(usingStatement.ResourceAcquisition) + } + }.CopyAnnotationsFrom(node); + } + } return usingStatement; } return null; } + + VariableDeclarationStatement FindVariableDeclaration(AstNode node, string identifier) + { + while (node != null) { + while (node.PrevSibling != null) { + node = node.PrevSibling; + VariableDeclarationStatement varDecl = node as VariableDeclarationStatement; + if (varDecl != null && varDecl.Variables.Count == 1 && varDecl.Variables.Single().Name == identifier) { + return varDecl; + } + } + node = node.Parent; + } + return null; + } + + bool CanMoveVariableDeclarationIntoStatement(VariableDeclarationStatement varDecl, Statement targetStatement, out Statement declarationPoint) + { + Debug.Assert(targetStatement.Ancestors.Contains(varDecl.Parent)); + // Find all blocks between targetStatement and varDecl.Parent + List blocks = targetStatement.Ancestors.TakeWhile(block => block != varDecl.Parent).OfType().ToList(); + blocks.Add((BlockStatement)varDecl.Parent); // also handle the varDecl.Parent block itself + blocks.Reverse(); // go from parent blocks to child blocks + DefiniteAssignmentAnalysis daa = new DefiniteAssignmentAnalysis(blocks[0], context.CancellationToken); + declarationPoint = null; + foreach (BlockStatement block in blocks) { + if (!DeclareVariables.FindDeclarationPoint(daa, varDecl, block, out declarationPoint)) { + return false; + } + } + return true; + } #endregion #region foreach @@ -190,19 +242,22 @@ namespace ICSharpCode.Decompiler.Ast.Transforms }, EmbeddedStatement = new BlockStatement { new Repeat( - new NamedNode("variableDeclaration", new VariableDeclarationStatement { Type = new AnyNode(), Variables = { new AnyNode() } }) + new VariableDeclarationStatement { Type = new AnyNode(), Variables = { new VariableInitializer() } }.WithName("variablesOutsideLoop") ).ToStatement(), new WhileStatement { Condition = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Invoke("MoveNext"), EmbeddedStatement = new BlockStatement { + new Repeat( + new VariableDeclarationStatement { Type = new AnyNode(), Variables = { new VariableInitializer() } }.WithName("variablesInsideLoop") + ).ToStatement(), new AssignmentExpression { - Left = new NamedNode("itemVariable", new IdentifierExpression()), + Left = new IdentifierExpression().WithName("itemVariable"), Operator = AssignmentOperatorType.Assign, Right = new IdentifierExpressionBackreference("enumeratorVariable").ToExpression().Member("Current") }, new Repeat(new AnyNode("statement")).ToStatement() } - } + }.WithName("loop") }}; public ForeachStatement TransformForeach(UsingStatement node) @@ -210,26 +265,45 @@ namespace ICSharpCode.Decompiler.Ast.Transforms Match m = foreachPattern.Match(node); if (m == null) return null; + if (!(node.Parent is BlockStatement) && m.Has("variablesOutsideLoop")) { + // if there are variables outside the loop, we need to put those into the parent block, and that won't work if the direct parent isn't a block + return null; + } VariableInitializer enumeratorVar = m.Get("enumeratorVariable").Single(); - ILVariable itemVar = m.Get("itemVariable").Single().Annotation(); - if (itemVar == null) + IdentifierExpression itemVar = m.Get("itemVariable").Single(); + WhileStatement loop = m.Get("loop").Single(); + + // Find the declaration of the item variable: + // Because we look only outside the loop, we won't make the mistake of moving a captured variable across the loop boundary + VariableDeclarationStatement itemVarDecl = FindVariableDeclaration(loop, itemVar.Identifier); + if (itemVarDecl == null || !(itemVarDecl.Parent is BlockStatement)) + return null; + + // Now verify that we can move the variable declaration in front of the loop: + Statement declarationPoint; + CanMoveVariableDeclarationIntoStatement(itemVarDecl, loop, out declarationPoint); + // We ignore the return value because we don't care whether we can move the variable into the loop + // (that is possible only with non-captured variables). + // We just care that we can move it in front of the loop: + if (declarationPoint != loop) return null; - return null; // TODO -// if (m.Has("itemVariableInsideLoop") && itemVar.Annotation() != null) { -// // cannot move captured variables out of loops -// return null; -// } + BlockStatement newBody = new BlockStatement(); + foreach (Statement stmt in m.Get("variablesInsideLoop")) + newBody.Add(stmt.Detach()); foreach (Statement stmt in m.Get("statement")) newBody.Add(stmt.Detach()); ForeachStatement foreachStatement = new ForeachStatement { - VariableType = m.Get("itemType").Single().Detach(), - VariableName = itemVar.Name, + VariableType = (AstType)itemVarDecl.Type.Clone(), + VariableName = itemVar.Identifier, InExpression = m.Get("collection").Single().Detach(), EmbeddedStatement = newBody }; node.ReplaceWith(foreachStatement); + foreach (Statement stmt in m.Get("variablesOutsideLoop")) { + ((BlockStatement)foreachStatement.Parent).Statements.InsertAfter(null, stmt); + } return foreachStatement; } #endregion diff --git a/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs b/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs index 57a5ada6c..6e2446559 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/TransformationPipeline.cs @@ -20,10 +20,10 @@ namespace ICSharpCode.Decompiler.Ast.Transforms new PushNegation(), new DelegateConstruction(context), new PatternStatementTransform(context), - new ConvertConstructorCallIntoInitializer(), new ReplaceMethodCallsWithOperators(), new IntroduceUnsafeModifier(), - new DeclareVariables(context), + new DeclareVariables(context), // should run after most transforms that modify statements + new ConvertConstructorCallIntoInitializer(), // must run after DeclareVariables new IntroduceUsingDeclarations(context) }; } From cca75477891cb342a5ed42e3fbf258260aa01871 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 14:46:28 +0100 Subject: [PATCH 08/10] Fixed some bugs in DeclareVariables. --- .../Ast/AstMethodBodyBuilder.cs | 4 +- .../Ast/Transforms/DeclareVariables.cs | 45 +++++++++++++++---- .../Ast/Transforms/DelegateConstruction.cs | 6 +-- .../Transforms/PatternStatementTransform.cs | 20 ++------- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs b/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs index 6bd62aad8..f36e02577 100644 --- a/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs +++ b/ICSharpCode.Decompiler/Ast/AstMethodBodyBuilder.cs @@ -88,9 +88,11 @@ namespace ICSharpCode.Decompiler.Ast context.CancellationToken.ThrowIfCancellationRequested(); Ast.BlockStatement astBlock = TransformBlock(ilMethod); CommentStatement.ReplaceAll(astBlock); // convert CommentStatements to Comments + + Statement insertionPoint = astBlock.Statements.FirstOrDefault(); foreach (ILVariable v in localVariablesToDefine) { var newVarDecl = new VariableDeclarationStatement(AstBuilder.ConvertType(v.Type), v.Name); - astBlock.Statements.InsertAfter(null, newVarDecl); + astBlock.Statements.InsertBefore(insertionPoint, newVarDecl); } return astBlock; diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs index fe2cba560..818504751 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs @@ -92,14 +92,22 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (TryConvertAssignmentExpressionIntoVariableDeclaration((Expression)usingStmt.ResourceAcquisition, type, variableName)) continue; } - foreach (BlockStatement subBlock in stmt.Children.OfType()) { - DeclareVariableInBlock(daa, subBlock, type, variableName, allowPassIntoLoops); + foreach (AstNode child in stmt.Children) { + BlockStatement subBlock = child as BlockStatement; + if (subBlock != null) { + DeclareVariableInBlock(daa, subBlock, type, variableName, allowPassIntoLoops); + } else if (HasNestedBlocks(child)) { + foreach (BlockStatement nestedSubBlock in child.Children.OfType()) { + DeclareVariableInBlock(daa, nestedSubBlock, type, variableName, allowPassIntoLoops); + } + } } } } else { // Try converting an assignment expression into a VariableDeclarationStatement if (!TryConvertAssignmentExpressionIntoVariableDeclaration(declarationPoint, type, variableName)) { // Declare the variable in front of declarationPoint + block.Statements.InsertBefore( declarationPoint, new VariableDeclarationStatement((AstType)type.Clone(), variableName) @@ -117,9 +125,11 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { IdentifierExpression ident = ae.Left as IdentifierExpression; if (ident != null && ident.Identifier == variableName) { + // We clone the right expression so that it doesn't get removed from the old ExpressionStatement, + // which might be still in use by the definite assignment graph. declarationPoint.ReplaceWith(new VariableDeclarationStatement { Type = (AstType)type.Clone(), - Variables = { new VariableInitializer(variableName, ae.Right.Detach()).CopyAnnotationsFrom(ae) } + Variables = { new VariableInitializer(variableName, ae.Right.Clone()).CopyAnnotationsFrom(ae) } }.CopyAnnotationsFrom(es).WithAnnotation(new DeclaredVariableAnnotation(es))); return true; } @@ -136,7 +146,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (ident != null && ident.Identifier == variableName) { expression.ReplaceWith(new VariableDeclarationStatement { Type = (AstType)type.Clone(), - Variables = { new VariableInitializer(variableName, ae.Right.Detach()).CopyAnnotationsFrom(ae) } + Variables = { new VariableInitializer(variableName, ae.Right.Clone()).CopyAnnotationsFrom(ae) } }.WithAnnotation(declaredVariableAnnotation)); return true; } @@ -179,7 +189,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms return false; } // If we can move the variable into the sub-block, we need to ensure that the remaining code - // does not use the value that was assigend by the first sub-block + // does not use the value that was assigned by the first sub-block Statement nextStatement = stmt.NextStatement; // The next statement might be a variable declaration that we inserted, and thus does not exist // in the definite assignment graph. Thus we need to look up the corresponding instruction @@ -239,14 +249,28 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } } - // We can move the variable into a sub-block only if the variable is used in only that sub-block + // We can move the variable into a sub-block only if the variable is used in only that sub-block (and not in expressions such as the loop condition) for (AstNode child = stmt.FirstChild; child != null; child = child.NextSibling) { - if (!(child is BlockStatement) && UsesVariable(child, variableName)) - return false; + if (!(child is BlockStatement) && UsesVariable(child, variableName)) { + if (HasNestedBlocks(child)) { + // catch clauses/switch sections can contain nested blocks + for (AstNode grandchild = child.FirstChild; grandchild != null; grandchild = grandchild.NextSibling) { + if (!(grandchild is BlockStatement) && UsesVariable(grandchild, variableName)) + return false; + } + } else { + return false; + } + } } return true; } + static bool HasNestedBlocks(AstNode node) + { + return node is CatchClause || node is SwitchSection; + } + static bool UsesVariable(AstNode node, string variableName) { IdentifierExpression ie = node as IdentifierExpression; @@ -278,6 +302,11 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } } + CatchClause catchClause = node as CatchClause; + if (catchClause != null && catchClause.VariableName == variableName) { + return false; // no need to introduce the variable here + } + for (AstNode child = node.FirstChild; child != null; child = child.NextSibling) { if (UsesVariable(child, variableName)) return true; diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs index b619230a9..639198232 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs @@ -276,11 +276,11 @@ namespace ICSharpCode.Decompiler.Ast.Transforms } } // Now insert the variable declarations (we can do this after the replacements only so that the scope detection works): + Statement insertionPoint = blockStatement.Statements.FirstOrDefault(); foreach (var tuple in variablesToDeclare) { var newVarDecl = new VariableDeclarationStatement(tuple.Item1, tuple.Item2); - if (newVarDecl != null) - newVarDecl.Variables.Single().AddAnnotation(new CapturedVariableAnnotation()); - blockStatement.Statements.InsertAfter(null, newVarDecl); + newVarDecl.Variables.Single().AddAnnotation(new CapturedVariableAnnotation()); + blockStatement.Statements.InsertBefore(insertionPoint, newVarDecl); } } return null; diff --git a/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs index 71c56e6a6..3b01fcb5d 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/PatternStatementTransform.cs @@ -445,19 +445,9 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (exit.Match(assign.Left) == null) return null; enter = assign.Right; - // Remove 'exit' variable: - bool ok = false; - for (AstNode tmp = node.NextSibling; tmp != tryCatch; tmp = tmp.NextSibling) { - VariableDeclarationStatement v = (VariableDeclarationStatement)tmp; - if (v.Variables.Single().Name == exit.Identifier) { - ok = true; - v.Remove(); - break; - } - } - if (!ok) - return null; + // TODO: verify that 'obj' variable can be removed } + // TODO: verify that 'flag' variable can be removed // transform the code into a lock statement: LockStatement l = new LockStatement(); l.Expression = enter.Detach(); @@ -487,17 +477,13 @@ namespace ICSharpCode.Decompiler.Ast.Transforms }, TrueStatement = new AnyNode("dictCreation") }, - new VariableDeclarationStatement { - Type = new PrimitiveType("int"), - Variables = { new NamedNode("intVar", new VariableInitializer()) } - }, new IfElseStatement { Condition = new Backreference("cachedDict").ToExpression().Invoke( "TryGetValue", new NamedNode("switchVar", new IdentifierExpression()), new DirectionExpression { FieldDirection = FieldDirection.Out, - Expression = new IdentifierExpressionBackreference("intVar") + Expression = new IdentifierExpression().WithName("intVar") }), TrueStatement = new BlockStatement { Statements = { From 6b638ec33089a0e3605bf954d0ab215e7f5a6885 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 15:36:41 +0100 Subject: [PATCH 09/10] DefiniteAssignmentAnalysis bugfix --- .../Analysis/DefiniteAssignmentAnalysis.cs | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs index 0c9b4ac28..d4cc6e1c5 100644 --- a/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs +++ b/NRefactory/ICSharpCode.NRefactory/CSharp/Analysis/DefiniteAssignmentAnalysis.cs @@ -123,6 +123,10 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis } // Verify that we created nodes for all statements: Debug.Assert(!rootStatement.DescendantsAndSelf.OfType().Except(allNodes.Select(n => n.NextStatement)).Any()); + // Verify that we put all nodes into the dictionaries: + Debug.Assert(rootStatement.DescendantsAndSelf.OfType().All(stmt => beginNodeDict.ContainsKey(stmt))); + Debug.Assert(rootStatement.DescendantsAndSelf.OfType().All(stmt => endNodeDict.ContainsKey(stmt))); + this.analyzedRangeStart = 0; this.analyzedRangeEnd = allNodes.Count - 1; } @@ -166,6 +170,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis /// Both 'start' and 'end' are inclusive. public void SetAnalyzedRange(Statement start, Statement end) { + Debug.Assert(beginNodeDict.ContainsKey(start) && endNodeDict.ContainsKey(end)); int startIndex = beginNodeDict[start].Index; int endIndex = endNodeDict[end].Index; if (startIndex > endIndex) @@ -353,27 +358,26 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis DefiniteAssignmentStatus oldStatus = edgeStatus[edge]; if (oldStatus == newStatus) return; - // Ensure that status can change only in one direction: - // CodeUnreachable -> PotentiallyAssigned -> DefinitelyAssigned - // Going against this direction indicates a bug and could cause infinite loops. - switch (oldStatus) { - case DefiniteAssignmentStatus.PotentiallyAssigned: - if (newStatus != DefiniteAssignmentStatus.DefinitelyAssigned) - throw new InvalidOperationException("Invalid state transition"); - break; - case DefiniteAssignmentStatus.CodeUnreachable: - if (!(newStatus == DefiniteAssignmentStatus.PotentiallyAssigned || newStatus == DefiniteAssignmentStatus.DefinitelyAssigned)) - throw new InvalidOperationException("Invalid state transition"); - break; - case DefiniteAssignmentStatus.DefinitelyAssigned: - throw new InvalidOperationException("Invalid state transition"); - default: - throw new InvalidOperationException("Invalid value for DefiniteAssignmentStatus"); + // Ensure that status can cannot change back to CodeUnreachable after it once was reachable. + // Also, don't ever use AssignedAfter... for statements. + if (newStatus == DefiniteAssignmentStatus.CodeUnreachable + || newStatus == DefiniteAssignmentStatus.AssignedAfterFalseExpression + || newStatus == DefiniteAssignmentStatus.AssignedAfterTrueExpression) + { + throw new InvalidOperationException(); } + // Note that the status can change from DefinitelyAssigned + // back to PotentiallyAssigned as unreachable input edges are + // discovered to be reachable. + edgeStatus[edge] = newStatus; DefiniteAssignmentNode targetNode = (DefiniteAssignmentNode)edge.To; - if (analyzedRangeStart <= targetNode.Index && targetNode.Index <= analyzedRangeEnd) + if (analyzedRangeStart <= targetNode.Index && targetNode.Index <= analyzedRangeEnd) { + // TODO: potential optimization: visit previously unreachable nodes with higher priority + // (e.g. use Deque and enqueue previously unreachable nodes at the front, but + // other nodes at the end) nodesWithModifiedInput.Enqueue(targetNode); + } } /// From 6a0d365fe41e69bf42fa98368536364ea6ebf420 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 20 Mar 2011 15:37:30 +0100 Subject: [PATCH 10/10] Fix crash in DeclareVariables when an anonymous method which contains variable declarations was used in the initializer of a variable declaration in the parent method. --- .../Ast/Transforms/DeclareVariables.cs | 83 +++++++++---------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs index 818504751..314c929b1 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/DeclareVariables.cs @@ -16,17 +16,17 @@ namespace ICSharpCode.Decompiler.Ast.Transforms /// public class DeclareVariables : IAstTransform { - sealed class DeclaredVariableAnnotation { - public readonly ExpressionStatement OriginalAssignmentStatement; + sealed class VariableToDeclare + { + public AstType Type; + public string Name; - public DeclaredVariableAnnotation(ExpressionStatement originalAssignmentStatement) - { - this.OriginalAssignmentStatement = originalAssignmentStatement; - } + public AssignmentExpression ReplacedAssignment; + public Statement InsertionPoint; } - static readonly DeclaredVariableAnnotation declaredVariableAnnotation = new DeclaredVariableAnnotation(null); readonly CancellationToken cancellationToken; + List variablesToDeclare = new List(); public DeclareVariables(DecompilerContext context) { @@ -36,14 +36,41 @@ namespace ICSharpCode.Decompiler.Ast.Transforms public void Run(AstNode node) { Run(node, null); + // Declare all the variables at the end, after all the logic has run. + // This is done so that definite assignment analysis can work on a single representation and doesn't have to be updated + // when we change the AST. + foreach (var v in variablesToDeclare) { + if (v.ReplacedAssignment == null) { + BlockStatement block = (BlockStatement)v.InsertionPoint.Parent; + block.Statements.InsertBefore( + v.InsertionPoint, + new VariableDeclarationStatement((AstType)v.Type.Clone(), v.Name)); + } + } + // First do all the insertions, then do all the replacements. This is necessary because a replacement might remove our reference point from the AST. + foreach (var v in variablesToDeclare) { + if (v.ReplacedAssignment != null) { + // We clone the right expression so that it doesn't get removed from the old ExpressionStatement, + // which might be still in use by the definite assignment graph. + VariableDeclarationStatement varDecl = new VariableDeclarationStatement { + Type = (AstType)v.Type.Clone(), + Variables = { new VariableInitializer(v.Name, v.ReplacedAssignment.Right.Detach()).CopyAnnotationsFrom(v.ReplacedAssignment) } + }; + ExpressionStatement es = v.ReplacedAssignment.Parent as ExpressionStatement; + if (es != null) + es.ReplaceWith(varDecl.CopyAnnotationsFrom(es)); + else + v.ReplacedAssignment.ReplaceWith(varDecl); + } + } + variablesToDeclare = null; } void Run(AstNode node, DefiniteAssignmentAnalysis daa) { BlockStatement block = node as BlockStatement; if (block != null) { - var variables = block.Statements.TakeWhile(stmt => stmt is VariableDeclarationStatement - && stmt.Annotation() == null) + var variables = block.Statements.TakeWhile(stmt => stmt is VariableDeclarationStatement) .Cast().ToList(); if (variables.Count > 0) { // remove old variable declarations: @@ -107,11 +134,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms // Try converting an assignment expression into a VariableDeclarationStatement if (!TryConvertAssignmentExpressionIntoVariableDeclaration(declarationPoint, type, variableName)) { // Declare the variable in front of declarationPoint - - block.Statements.InsertBefore( - declarationPoint, - new VariableDeclarationStatement((AstType)type.Clone(), variableName) - .WithAnnotation(declaredVariableAnnotation)); + variablesToDeclare.Add(new VariableToDeclare { Type = type, Name = variableName, InsertionPoint = declarationPoint }); } } } @@ -121,19 +144,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms // convert the declarationPoint into a VariableDeclarationStatement ExpressionStatement es = declarationPoint as ExpressionStatement; if (es != null) { - AssignmentExpression ae = es.Expression as AssignmentExpression; - if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { - IdentifierExpression ident = ae.Left as IdentifierExpression; - if (ident != null && ident.Identifier == variableName) { - // We clone the right expression so that it doesn't get removed from the old ExpressionStatement, - // which might be still in use by the definite assignment graph. - declarationPoint.ReplaceWith(new VariableDeclarationStatement { - Type = (AstType)type.Clone(), - Variables = { new VariableInitializer(variableName, ae.Right.Clone()).CopyAnnotationsFrom(ae) } - }.CopyAnnotationsFrom(es).WithAnnotation(new DeclaredVariableAnnotation(es))); - return true; - } - } + return TryConvertAssignmentExpressionIntoVariableDeclaration(es.Expression, type, variableName); } return false; } @@ -144,10 +155,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms if (ae != null && ae.Operator == AssignmentOperatorType.Assign) { IdentifierExpression ident = ae.Left as IdentifierExpression; if (ident != null && ident.Identifier == variableName) { - expression.ReplaceWith(new VariableDeclarationStatement { - Type = (AstType)type.Clone(), - Variables = { new VariableInitializer(variableName, ae.Right.Clone()).CopyAnnotationsFrom(ae) } - }.WithAnnotation(declaredVariableAnnotation)); + variablesToDeclare.Add(new VariableToDeclare { Type = type, Name = variableName, ReplacedAssignment = ae }); return true; } } @@ -191,19 +199,6 @@ namespace ICSharpCode.Decompiler.Ast.Transforms // If we can move the variable into the sub-block, we need to ensure that the remaining code // does not use the value that was assigned by the first sub-block Statement nextStatement = stmt.NextStatement; - // The next statement might be a variable declaration that we inserted, and thus does not exist - // in the definite assignment graph. Thus we need to look up the corresponding instruction - // prior to the introduction of the VariableDeclarationStatement. - while (nextStatement is VariableDeclarationStatement) { - DeclaredVariableAnnotation annotation = nextStatement.Annotation(); - if (annotation == null) - break; - if (annotation.OriginalAssignmentStatement != null) { - nextStatement = annotation.OriginalAssignmentStatement; - break; - } - nextStatement = nextStatement.NextStatement; - } if (nextStatement != null) { // Analyze the range from the next statement to the end of the block daa.SetAnalyzedRange(nextStatement, block);