Optimize renaming of declarations named after keywords

ddobrev · ddobrev · commit 6b3cdaffb7a5 · 2019-04-23T02:47:27.000+03:00
Signed-off-by: Dimitar Dobrev &lt;dpldobrev@protonmail.com&gt;
diff --git a/src/CppParser/Bootstrap/Bootstrap.cs b/src/CppParser/Bootstrap/Bootstrap.cs
@@ -1762,11 +1762,9 @@ public static string GetDeclName(Declaration decl,
                 }
 
                 if (!hasConflict)
-                    name = CaseRenamePass.ConvertCaseString(decl,
-                        RenameCasePattern.UpperCamelCase);
-
-                if (CSharpSources.IsReservedKeyword(name))
-                    name = $"@{name}";
+                    name = CSharpSources.SafeIdentifier(
+                        CaseRenamePass.ConvertCaseString(decl,
+                            RenameCasePattern.UpperCamelCase));
             }
             else throw new NotImplementedException();
 
diff --git a/src/Generator/Driver.cs b/src/Generator/Driver.cs
@@ -277,7 +277,10 @@ public void SetupPasses(ILibrary library)
 
             if (Options.GeneratorKind == GeneratorKind.CLI ||
                 Options.GeneratorKind == GeneratorKind.CSharp)
-                TranslationUnitPasses.RenameDeclsUpperCase(RenameTargets.Any &~ RenameTargets.Parameter);
+            {
+                TranslationUnitPasses.RenameDeclsUpperCase(RenameTargets.Any & ~RenameTargets.Parameter);
+                TranslationUnitPasses.AddPass(new CheckKeywordNamesPass());
+            }
         }
 
         public void ProcessCode()
diff --git a/src/Generator/Generators/C/CCodeGenerator.cs b/src/Generator/Generators/C/CCodeGenerator.cs
@@ -41,11 +41,6 @@ public virtual string QualifiedName(Declaration decl)
             return decl.QualifiedName;
         }
 
-        public override string GeneratedIdentifier(string id)
-        {
-            return "__" + id.Replace('-', '_');
-        }
-
         private CppTypePrinter typePrinter = new CppTypePrinter();
         public virtual CppTypePrinter CTypePrinter => typePrinter;
 
@@ -186,7 +181,7 @@ public override void GenerateClassSpecifier(Class @class)
 
             if (Options.GeneratorKind == GeneratorKind.CLI)
             {
-                keywords.Add(AccessIdentifier(@class.Access));
+                keywords.Add(Helpers.GetAccess(@class.Access));
 
                 if (@class.IsAbstract)
                     keywords.Add("abstract");
diff --git a/src/Generator/Generators/CSharp/CSharpSources.cs b/src/Generator/Generators/CSharp/CSharpSources.cs
@@ -59,9 +59,7 @@ private void Init()
             "void", "partial", "yield", "where"
         };
 
-        public static bool IsReservedKeyword(string id) => ReservedKeywords.Contains(id);
-
-        public override string SafeIdentifier(string id)
+        public static string SafeIdentifier(string id)
         {
             if (id.All(char.IsLetterOrDigit))
                 return ReservedKeywords.Contains(id) ? "@" + id : id;
@@ -246,7 +244,7 @@ public virtual void GenerateNamespaceFunctionsAndVariables(DeclarationContext co
                 return;
 
             PushBlock(BlockKind.Functions);
-            var parentName = SafeIdentifier(context.TranslationUnit.FileNameWithoutExtension);
+            var parentName = context.TranslationUnit.FileNameWithoutExtension;
 
             var keyword = "class";
             var classes = EnumerateClasses().ToList();
@@ -724,7 +722,7 @@ public override void GenerateClassSpecifier(Class @class)
             keywords.Add("partial");
 
             keywords.Add(@class.IsInterface ? "interface" : (@class.IsValueType ? "struct" : "class"));
-            keywords.Add(SafeIdentifier(@class.Name));
+            keywords.Add(@class.Name);
 
             Write(string.Join(" ", keywords));
             if (@class.IsDependent && @class.TemplateParameters.Any())
@@ -927,7 +925,7 @@ private void GenerateFieldSetter(Field field, Class @class, QualifiedType fieldT
             else
             {
                 var name = @class.Layout.Fields.First(f => f.FieldPtr == field.OriginalPtr).Name;
-                var identifier = SafeIdentifier(name);
+                var identifier = name;
                 if (@class.IsValueType)
                     returnVar = $"{Helpers.InstanceField}.{identifier}";
                 else
@@ -998,7 +996,7 @@ private string HandleValueArray(ArrayType arrayType, Field field)
             var name = ((Class) field.Namespace).Layout.Fields.First(
                 f => f.FieldPtr == field.OriginalPtr).Name;
             WriteLine(string.Format("fixed ({0} {1} = {2}.{3})",
-                type, arrPtr, Helpers.InstanceField, SafeIdentifier(name)));
+                type, arrPtr, Helpers.InstanceField, name));
             WriteOpenBraceAndIndent();
             return arrPtr;
         }
@@ -1219,11 +1217,11 @@ private void GenerateFieldGetter(Field field, Class @class, QualifiedType return
                 if (arrayType != null)
                     returnVar = HandleValueArray(arrayType, field);
                 else
-                    returnVar = $"{Helpers.InstanceField}.{SafeIdentifier(name)}";
+                    returnVar = $"{Helpers.InstanceField}.{name}";
             }
             else
             {
-                returnVar = $"(({TypePrinter.PrintNative(@class)}*) {Helpers.InstanceIdentifier})->{SafeIdentifier(name)}";
+                returnVar = $"(({TypePrinter.PrintNative(@class)}*) {Helpers.InstanceIdentifier})->{name}";
                 // Class field getter should return a reference object instead of a copy. Wrapping `returnVar` in
                 // IntPtr ensures that non-copying object constructor is invoked.
                 Class typeClass;
@@ -1372,8 +1370,8 @@ private void GenerateProperties(Class @class)
                 {
                     var name = @class.Layout.Fields.First(f => f.FieldPtr == prop.Field.OriginalPtr).Name;
                     GenerateClassField(prop.Field);
-                    WriteLine("private bool {0};",
-                        GeneratedIdentifier(string.Format("{0}Initialised", name)));
+                    string safeIdentifier = name.StartsWith("@") ? name.Substring(1) : name;
+                    WriteLine($"private bool __{safeIdentifier}Initialised;");
                 }
 
                 GenerateDeclarationCommon(prop);
@@ -1434,7 +1432,7 @@ private string GetPropertyName(Property prop)
         {
             var isIndexer = prop.Parameters.Count != 0;
             if (!isIndexer)
-                return SafeIdentifier(prop.Name);
+                return prop.Name;
 
             var @params = prop.Parameters.Select(param => {
                 var p = new Parameter(param);
@@ -3109,7 +3107,7 @@ public override bool VisitEnumDecl(Enumeration @enum)
             // internal P/Invoke declarations must see protected enums
             if (@enum.Access == AccessSpecifier.Protected)
                 Write("internal ");
-            Write("enum {0}", SafeIdentifier(@enum.Name));
+            Write("enum {0}", @enum.Name);
 
             var typeName = TypePrinter.VisitPrimitiveType(@enum.BuiltinType.Type,
                                                           new TypeQualifiers());
diff --git a/src/Generator/Generators/CSharp/CSharpTypePrinter.cs b/src/Generator/Generators/CSharp/CSharpTypePrinter.cs
@@ -739,22 +739,16 @@ private static string GetParameterUsage(ParameterUsage usage)
         public override TypePrinterResult VisitFieldDecl(Field field)
         {
             var cSharpSourcesDummy = new CSharpSources(Context, new List<TranslationUnit>());
-            var safeIdentifier = cSharpSourcesDummy.SafeIdentifier(field.Name);
 
-            if (safeIdentifier.All(c => c.Equals('_')))
-            {
-                safeIdentifier = cSharpSourcesDummy.SafeIdentifier(field.Name);
-            }
-
-            PushMarshalKind(MarshalKind.NativeField);   
+            PushMarshalKind(MarshalKind.NativeField);
             var fieldTypePrinted = field.QualifiedType.Visit(this);
             PopMarshalKind();
-                     
+
             var returnTypePrinter = new TypePrinterResult();
             if (!string.IsNullOrWhiteSpace(fieldTypePrinted.NameSuffix))
                 returnTypePrinter.NameSuffix = fieldTypePrinted.NameSuffix;
 
-            returnTypePrinter.Type = $"{fieldTypePrinted.Type} {safeIdentifier}"; 
+            returnTypePrinter.Type = $"{fieldTypePrinted.Type} {field.Name}"; 
 
             return returnTypePrinter;
         }
diff --git a/src/Generator/Generators/CodeGenerator.cs b/src/Generator/Generators/CodeGenerator.cs
@@ -105,19 +105,6 @@ public virtual void GenerateDebug(Declaration decl)
 
         #endregion
 
-        #region Identifiers generation
-
-        public virtual string SafeIdentifier(string id) => id;
-
-        public virtual string GeneratedIdentifier(string id) => Generator.GeneratedIdentifier(id);
-
-        public virtual string AccessIdentifier(AccessSpecifier accessSpecifier)
-        {
-            return Helpers.GetAccess(accessSpecifier);
-        }
-
-        #endregion
-
         #region Comment generation
 
         public virtual void GenerateSummary(string comment)
diff --git a/src/Generator/Passes/CheckKeywordNamesPass.cs b/src/Generator/Passes/CheckKeywordNamesPass.cs
@@ -0,0 +1,31 @@
+﻿using CppSharp.AST;
+using CppSharp.Generators.CSharp;
+
+namespace CppSharp.Passes
+{
+    public class CheckKeywordNamesPass : TranslationUnitPass
+    {
+        public override bool VisitClassDecl(Class @class)
+        {
+            if (!base.VisitClassDecl(@class) || @class.Layout == null)
+                return false;
+
+            foreach (var field in @class.Layout.Fields)
+                field.Name = SafeIdentifier(field.Name);
+
+            return true;
+        }
+
+        public override bool VisitDeclaration(Declaration decl)
+        {
+            if (!base.VisitDeclaration(decl) || decl.Ignore)
+                return false;
+
+            decl.Name = SafeIdentifier(decl.Name);
+            return true;
+        }
+
+        private string SafeIdentifier(string id) =>
+            Options.IsCLIGenerator ? id : CSharpSources.SafeIdentifier(id);
+    }
+}
diff --git a/src/Generator/Passes/CleanInvalidDeclNamesPass.cs b/src/Generator/Passes/CleanInvalidDeclNamesPass.cs
@@ -2,23 +2,12 @@
 using System.Linq;
 using System.Text;
 using CppSharp.AST;
-using CppSharp.Generators.CLI;
-using CppSharp.Generators.CSharp;
 using CppSharp.Generators;
 
 namespace CppSharp.Passes
 {
     public class CleanInvalidDeclNamesPass : TranslationUnitPass
     {
-        public override bool VisitASTContext(ASTContext context)
-        {
-            // TODO: Fix this to not need per-generator code.
-            generator = Options.IsCLIGenerator ?
-               new CLIHeaders(Context, new List<TranslationUnit>()) :
-               (CodeGenerator) new CSharpSources(Context);
-            return base.VisitASTContext(context);
-        }
-
         public override bool VisitClassDecl(Class @class)
         {
             if (!base.VisitClassDecl(@class))
@@ -138,10 +127,8 @@ private string CheckName(string name, ref int order)
             if (char.IsNumber(name[0]))
                 return '_' + name;
 
-            return generator.SafeIdentifier(name);
+            return name;
         }
-
-        private CodeGenerator generator;
     }
 }
 
diff --git a/src/Generator/Passes/SpecializationMethodsWithDependentPointersPass.cs b/src/Generator/Passes/SpecializationMethodsWithDependentPointersPass.cs
@@ -129,7 +129,7 @@ private static Method GetExtensionMethodForDependentPointer(Method specializedMe
                     var thisParameter = new Parameter();
                     thisParameter.QualifiedType = new QualifiedType(new PointerType(
                         new QualifiedType(new TagType(specializedMethod.Namespace))));
-                    thisParameter.Name = "@this";
+                    thisParameter.Name = "this";
                     thisParameter.Kind = ParameterKind.Extension;
                     thisParameter.Namespace = extensionMethod;
                     extensionMethod.Parameters.Insert(0, thisParameter);

Original file line number	Diff line number	Diff line change
`@@ -1762,11 +1762,9 @@ public static string GetDeclName(Declaration decl,`
`1762`	`1762`	`}`
`1763`	`1763`
`1764`	`1764`	`if (!hasConflict)`
`1765`		`- name = CaseRenamePass.ConvertCaseString(decl,`
`1766`		`- RenameCasePattern.UpperCamelCase);`
`1767`		`-`
`1768`		`- if (CSharpSources.IsReservedKeyword(name))`
`1769`		`- name = $"@{name}";`
	`1765`	`+ name = CSharpSources.SafeIdentifier(`
	`1766`	`+ CaseRenamePass.ConvertCaseString(decl,`
	`1767`	`+ RenameCasePattern.UpperCamelCase));`
`1770`	`1768`	`}`
`1771`	`1769`	`else throw new NotImplementedException();`
`1772`	`1770`
Original file line number	Diff line number	Diff line change
`@@ -2,23 +2,12 @@`
`2`	`2`	`using System.Linq;`
`3`	`3`	`using System.Text;`
`4`	`4`	`using CppSharp.AST;`
`5`		`-using CppSharp.Generators.CLI;`
`6`		`-using CppSharp.Generators.CSharp;`
`7`	`5`	`using CppSharp.Generators;`
`8`	`6`
`9`	`7`	`namespace CppSharp.Passes`
`10`	`8`	`{`
`11`	`9`	`public class CleanInvalidDeclNamesPass : TranslationUnitPass`
`12`	`10`	`{`
`13`		`- public override bool VisitASTContext(ASTContext context)`
`14`		`- {`
`15`		`- // TODO: Fix this to not need per-generator code.`
`16`		`- generator = Options.IsCLIGenerator ?`
`17`		`- new CLIHeaders(Context, new List<TranslationUnit>()) :`
`18`		`- (CodeGenerator) new CSharpSources(Context);`
`19`		`- return base.VisitASTContext(context);`
`20`		`- }`
`21`		`-`
`22`	`11`	`public override bool VisitClassDecl(Class @class)`
`23`	`12`	`{`
`24`	`13`	`if (!base.VisitClassDecl(@class))`
`@@ -138,10 +127,8 @@ private string CheckName(string name, ref int order)`
`138`	`127`	`if (char.IsNumber(name[0]))`
`139`	`128`	`return '_' + name;`
`140`	`129`
`141`		`- return generator.SafeIdentifier(name);`
	`130`	`+ return name;`
`142`	`131`	`}`
`143`		`-`
`144`		`- private CodeGenerator generator;`
`145`	`132`	`}`
`146`	`133`	`}`
`147`	`134`