From 69ac94363d0f4340e4fe9ae5b8cbf318b2548e4c Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 9 Apr 2011 15:48:29 +0200 Subject: [PATCH] Resolve variable naming conflicts that occur with anonymous methods. --- .../Ast/DecompilerContext.cs | 10 +- ICSharpCode.Decompiler/Ast/NameVariables.cs | 85 ++++++++---- .../Ast/Transforms/DelegateConstruction.cs | 128 ++++++++++++++++++ .../Tests/DelegateConstruction.cs | 41 ++++++ 4 files changed, 239 insertions(+), 25 deletions(-) diff --git a/ICSharpCode.Decompiler/Ast/DecompilerContext.cs b/ICSharpCode.Decompiler/Ast/DecompilerContext.cs index 69e098531..1028a4d4d 100644 --- a/ICSharpCode.Decompiler/Ast/DecompilerContext.cs +++ b/ICSharpCode.Decompiler/Ast/DecompilerContext.cs @@ -2,6 +2,7 @@ // This code is distributed under MIT X11 license (for details please see \doc\license.txt) using System; +using System.Collections.Generic; using System.Threading; using Mono.Cecil; @@ -14,9 +15,16 @@ namespace ICSharpCode.Decompiler public MethodDefinition CurrentMethod; public DecompilerSettings Settings = new DecompilerSettings(); + /// + /// Used to pass variable names from a method to its anonymous methods. + /// + internal List ReservedVariableNames = new List(); + public DecompilerContext Clone() { - return (DecompilerContext)MemberwiseClone(); + DecompilerContext ctx = (DecompilerContext)MemberwiseClone(); + ctx.ReservedVariableNames = new List(ctx.ReservedVariableNames); + return ctx; } } } diff --git a/ICSharpCode.Decompiler/Ast/NameVariables.cs b/ICSharpCode.Decompiler/Ast/NameVariables.cs index 5bc16c0a9..338f0bd20 100644 --- a/ICSharpCode.Decompiler/Ast/NameVariables.cs +++ b/ICSharpCode.Decompiler/Ast/NameVariables.cs @@ -36,8 +36,16 @@ namespace ICSharpCode.Decompiler.Ast NameVariables nv = new NameVariables(); nv.context = context; nv.fieldNamesInCurrentType = context.CurrentType.Fields.Select(f => f.Name).ToList(); - nv.AddExistingNames(parameters.Select(p => p.Name)); - nv.AddExistingNames(variables.Where(v => v.IsGenerated).Select(v => v.Name)); + // First mark existing variable names as reserved. + foreach (string name in context.ReservedVariableNames) + nv.AddExistingName(name); + foreach (var p in parameters) + nv.AddExistingName(p.Name); + foreach (var v in variables) { + if (v.IsGenerated) + nv.AddExistingName(v.Name); + } + // Now generate names: foreach (ILVariable p in parameters) { if (string.IsNullOrEmpty(p.Name)) p.Name = nv.GenerateNameForVariable(p, methodBody); @@ -53,30 +61,59 @@ namespace ICSharpCode.Decompiler.Ast List fieldNamesInCurrentType; Dictionary typeNames = new Dictionary(); - void AddExistingNames(IEnumerable existingNames) + public void AddExistingName(string name) { - foreach (string name in existingNames) { - if (string.IsNullOrEmpty(name)) - continue; - // First, identify whether the name already ends with a number: - int pos = name.Length; - while (pos > 0 && name[pos-1] >= '0' && name[pos-1] <= '9') - pos--; - if (pos < name.Length) { - int number; - if (int.TryParse(name.Substring(pos), out number)) { - string nameWithoutDigits = name.Substring(0, pos); - int existingNumber; - if (typeNames.TryGetValue(nameWithoutDigits, out existingNumber)) { - typeNames[nameWithoutDigits] = Math.Max(number, existingNumber); - } else { - typeNames.Add(nameWithoutDigits, number); - } - continue; + if (string.IsNullOrEmpty(name)) + return; + int number; + string nameWithoutDigits = SplitName(name, out number); + int existingNumber; + if (typeNames.TryGetValue(nameWithoutDigits, out existingNumber)) { + typeNames[nameWithoutDigits] = Math.Max(number, existingNumber); + } else { + typeNames.Add(nameWithoutDigits, number); + } + } + + string SplitName(string name, out int number) + { + // First, identify whether the name already ends with a number: + int pos = name.Length; + while (pos > 0 && name[pos-1] >= '0' && name[pos-1] <= '9') + pos--; + if (pos < name.Length) { + if (int.TryParse(name.Substring(pos), out number)) { + return name.Substring(0, pos); + } + } + number = 1; + return name; + } + + const char maxLoopVariableName = 'n'; + + public string GetAlternativeName(string oldVariableName) + { + if (oldVariableName.Length == 1 && oldVariableName[0] >= 'i' && oldVariableName[0] <= maxLoopVariableName) { + for (char c = 'i'; c <= maxLoopVariableName; c++) { + if (!typeNames.ContainsKey(c.ToString())) { + typeNames.Add(c.ToString(), 1); + return c.ToString(); } } - if (!typeNames.ContainsKey(name)) - typeNames.Add(name, 1); + } + + int number; + string nameWithoutDigits = SplitName(oldVariableName, out number); + + if (!typeNames.ContainsKey(nameWithoutDigits)) { + typeNames.Add(nameWithoutDigits, 0); + } + int count = ++typeNames[nameWithoutDigits]; + if (count > 1) { + return nameWithoutDigits + count.ToString(); + } else { + return nameWithoutDigits; } } @@ -106,7 +143,7 @@ namespace ICSharpCode.Decompiler.Ast } if (isLoopCounter) { // For loop variables, use i,j,k,l,m,n - for (char c = 'i'; c <= 'n'; c++) { + for (char c = 'i'; c <= maxLoopVariableName; c++) { if (!typeNames.ContainsKey(c.ToString())) { proposedName = c.ToString(); break; diff --git a/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs index ec7988bdd..0d584c3f4 100644 --- a/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/Ast/Transforms/DelegateConstruction.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Threading; using ICSharpCode.Decompiler; @@ -37,6 +38,8 @@ namespace ICSharpCode.Decompiler.Ast.Transforms { } + List currentlyUsedVariableNames = new List(); + public DelegateConstruction(DecompilerContext context) : base(context) { } @@ -119,10 +122,16 @@ namespace ICSharpCode.Decompiler.Ast.Transforms ame.Parameters.AddRange(AstBuilder.MakeParameters(method.Parameters)); ame.HasParameterList = true; + // rename variables so that they don't conflict with the parameters: + foreach (ParameterDeclaration pd in ame.Parameters) { + EnsureVariableNameIsAvailable(objectCreateExpression, pd.Name); + } + // Decompile the anonymous method: DecompilerContext subContext = context.Clone(); subContext.CurrentMethod = method; + subContext.ReservedVariableNames.AddRange(currentlyUsedVariableNames); BlockStatement body = AstMethodBodyBuilder.CreateMethodBody(method, subContext, ame.Parameters); TransformationPipeline.RunTransformationsUntil(body, v => v is DelegateConstruction, subContext); body.AcceptVisitor(this, null); @@ -179,6 +188,76 @@ namespace ICSharpCode.Decompiler.Ast.Transforms return true; } + #region Track current variables + public override object VisitMethodDeclaration(MethodDeclaration methodDeclaration, object data) + { + Debug.Assert(currentlyUsedVariableNames.Count == 0); + try { + currentlyUsedVariableNames.AddRange(methodDeclaration.Parameters.Select(p => p.Name)); + return base.VisitMethodDeclaration(methodDeclaration, data); + } finally { + currentlyUsedVariableNames.Clear(); + } + } + + public override object VisitOperatorDeclaration(OperatorDeclaration operatorDeclaration, object data) + { + Debug.Assert(currentlyUsedVariableNames.Count == 0); + try { + currentlyUsedVariableNames.AddRange(operatorDeclaration.Parameters.Select(p => p.Name)); + return base.VisitOperatorDeclaration(operatorDeclaration, data); + } finally { + currentlyUsedVariableNames.Clear(); + } + } + + public override object VisitConstructorDeclaration(ConstructorDeclaration constructorDeclaration, object data) + { + Debug.Assert(currentlyUsedVariableNames.Count == 0); + try { + currentlyUsedVariableNames.AddRange(constructorDeclaration.Parameters.Select(p => p.Name)); + return base.VisitConstructorDeclaration(constructorDeclaration, data); + } finally { + currentlyUsedVariableNames.Clear(); + } + } + + public override object VisitIndexerDeclaration(IndexerDeclaration indexerDeclaration, object data) + { + Debug.Assert(currentlyUsedVariableNames.Count == 0); + try { + currentlyUsedVariableNames.AddRange(indexerDeclaration.Parameters.Select(p => p.Name)); + return base.VisitIndexerDeclaration(indexerDeclaration, data); + } finally { + currentlyUsedVariableNames.Clear(); + } + } + + public override object VisitAccessor(Accessor accessor, object data) + { + try { + currentlyUsedVariableNames.Add("value"); + return base.VisitAccessor(accessor, data); + } finally { + currentlyUsedVariableNames.RemoveAt(currentlyUsedVariableNames.Count - 1); + } + } + + public override object VisitVariableDeclarationStatement(VariableDeclarationStatement variableDeclarationStatement, object data) + { + foreach (VariableInitializer v in variableDeclarationStatement.Variables) + currentlyUsedVariableNames.Add(v.Name); + return base.VisitVariableDeclarationStatement(variableDeclarationStatement, data); + } + + public override object VisitFixedStatement(FixedStatement fixedStatement, object data) + { + foreach (VariableInitializer v in fixedStatement.Variables) + currentlyUsedVariableNames.Add(v.Name); + return base.VisitFixedStatement(fixedStatement, data); + } + #endregion + static readonly ExpressionStatement displayClassAssignmentPattern = new ExpressionStatement(new AssignmentExpression( new NamedNode("variable", new IdentifierExpression()), @@ -187,6 +266,7 @@ namespace ICSharpCode.Decompiler.Ast.Transforms public override object VisitBlockStatement(BlockStatement blockStatement, object data) { + int numberOfVariablesOutsideBlock = currentlyUsedVariableNames.Count; base.VisitBlockStatement(blockStatement, data); foreach (ExpressionStatement stmt in blockStatement.Statements.OfType().ToArray()) { Match displayClassAssignmentMatch = displayClassAssignmentPattern.Match(stmt); @@ -261,6 +341,8 @@ namespace ICSharpCode.Decompiler.Ast.Transforms foreach (FieldDefinition field in type.Fields) { if (dict.ContainsKey(field)) continue; + EnsureVariableNameIsAvailable(blockStatement, field.Name); + currentlyUsedVariableNames.Add(field.Name); variablesToDeclare.Add(Tuple.Create(AstBuilder.ConvertType(field.FieldType, field), field.Name)); dict[field] = new IdentifierExpression(field.Name); } @@ -283,7 +365,53 @@ namespace ICSharpCode.Decompiler.Ast.Transforms blockStatement.Statements.InsertBefore(insertionPoint, newVarDecl); } } + currentlyUsedVariableNames.RemoveRange(numberOfVariablesOutsideBlock, currentlyUsedVariableNames.Count - numberOfVariablesOutsideBlock); return null; } + + void EnsureVariableNameIsAvailable(AstNode currentNode, string name) + { + int pos = currentlyUsedVariableNames.IndexOf(name); + if (pos < 0) { + // name is still available + return; + } + // Naming conflict. Let's rename the existing variable so that the field keeps the name from metadata. + NameVariables nv = new NameVariables(); + // Add currently used variable and parameter names + foreach (string nameInUse in currentlyUsedVariableNames) + nv.AddExistingName(nameInUse); + // variables declared in child nodes of this block + foreach (VariableInitializer vi in currentNode.Descendants.OfType()) + nv.AddExistingName(vi.Name); + // parameters in child lambdas + foreach (ParameterDeclaration pd in currentNode.Descendants.OfType()) + nv.AddExistingName(pd.Name); + + string newName = nv.GetAlternativeName(name); + currentlyUsedVariableNames[pos] = newName; + + // find top-most block + AstNode topMostBlock = currentNode.Ancestors.OfType().LastOrDefault() ?? currentNode; + + // rename identifiers + foreach (IdentifierExpression ident in topMostBlock.Descendants.OfType()) { + if (ident.Identifier == name) { + ident.Identifier = newName; + ILVariable v = ident.Annotation(); + if (v != null) + v.Name = newName; + } + } + // rename variable declarations + foreach (VariableInitializer vi in topMostBlock.Descendants.OfType()) { + if (vi.Name == name) { + vi.Name = newName; + ILVariable v = vi.Annotation(); + if (v != null) + v.Name = newName; + } + } + } } } diff --git a/ICSharpCode.Decompiler/Tests/DelegateConstruction.cs b/ICSharpCode.Decompiler/Tests/DelegateConstruction.cs index 74d69aa0d..1060215a1 100644 --- a/ICSharpCode.Decompiler/Tests/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/Tests/DelegateConstruction.cs @@ -77,4 +77,45 @@ public static class DelegateConstruction Console.WriteLine(); }; } + + public static void NameConflict() + { + // i is captured variable, + // j is parameter in anonymous method + // k is local in anonymous method, + // l is local in main method + // Ensure that the decompiler doesn't introduce name conflicts + List> list = new List>(); + for (int l = 0; l < 10; l++) { + int i; + for (i = 0; i < 10; i++) { + list.Add( + delegate (int j) { + for (int k = 0; k < i; k += j) { + Console.WriteLine(); + } + }); + } + } + } + + public static void NameConflict2(int j) + { + List> list = new List>(); + for (int k = 0; k < 10; k++) { + list.Add( + delegate(int i) { + Console.WriteLine(i); + }); + } + } + + public static Action NameConflict3(int i) + { + return delegate(int j) { + for (int k = 0; k < j; k++) { + Console.WriteLine(k); + } + }; + } }