Bug fixes for GCC 11, cache conversion robustness and error handling (mono#1765)

* Fix missing Attribute dtor in GCC 11.

Parser bindings are expecting this, yet this was optimized out under GCC
11.

* Improve error handling for failed library parsing.

* Make the converted declaration cache more robust.

We were getting a failure due to duplicated original pointers. Make it
take the declaration kind into account as a key to the cache.

* Change ConsoleDriver.Run to return a failure bool.

Author: tritao

src/CppParser/AST.cpp

@@ -1078,6 +1078,8 @@ HTMLStartTagComment::Attribute::Attribute() {}
 
 HTMLStartTagComment::Attribute::Attribute(const Attribute& rhs) : name(rhs.name), value(rhs.value) {}
 
+HTMLStartTagComment::Attribute::~Attribute() {}
+
 HTMLStartTagComment::HTMLStartTagComment() : HTMLTagComment(CommentKind::HTMLStartTagComment) {}
 
 DEF_VECTOR(HTMLStartTagComment, HTMLStartTagComment::Attribute, Attributes)

src/CppParser/AST.h

@@ -201,6 +201,7 @@ class CS_API HTMLStartTagComment : public HTMLTagComment
     public:
         Attribute();
         Attribute(const Attribute&);
+        ~Attribute();
         std::string name;
         std::string value;
     };

src/CppParser/CppParser.h

@@ -82,9 +82,9 @@ struct CS_API ParserDiagnostic
     ~ParserDiagnostic();
     std::string fileName;
     std::string message;
-    ParserDiagnosticLevel level;
-    int lineNumber;
-    int columnNumber;
+    ParserDiagnosticLevel level { ParserDiagnosticLevel::Ignored };
+    int lineNumber {0};
+    int columnNumber {0};
 };
 
 enum class ParserResultKind

src/CppParser/Parser.cpp

@@ -4713,7 +4713,13 @@ ParserResult* Parser::ParseLibrary(const CppLinkerOptions* Opts)
         auto BinaryOrErr = llvm::object::createBinary(FileEntry);
         if (!BinaryOrErr)
         {
-            auto Error = BinaryOrErr.takeError();
+            auto ErrMsg = llvm::toString(BinaryOrErr.takeError());
+            auto Diag = ParserDiagnostic();
+            Diag.fileName = FileEntry;
+            Diag.message = ErrMsg;
+            Diag.level = ParserDiagnosticLevel::Error;
+            res->Diagnostics.push_back(Diag);
+
             res->kind = ParserResultKind::Error;
             return res;
         }

src/Generator/Driver.cs

@@ -137,9 +137,17 @@ void OnFileParsed(IEnumerable<string> files, ParserResult result)
                 if (diag.Level == ParserDiagnosticLevel.Note)
                     continue;
 
-                Diagnostics.Message("{0}({1},{2}): {3}: {4}",
-                    diag.FileName, diag.LineNumber, diag.ColumnNumber,
-                    diag.Level.ToString().ToLower(), diag.Message);
+                if (diag.LineNumber == 0 && diag.ColumnNumber == 0)
+                {
+                    Diagnostics.Message("{0}: {1}: {2}",
+                        diag.FileName, diag.Level.ToString().ToLower(), diag.Message);
+                }
+                else
+                {
+                    Diagnostics.Message("{0}({1},{2}): {3}: {4}",
+                        diag.FileName, diag.LineNumber, diag.ColumnNumber,
+                        diag.Level.ToString().ToLower(), diag.Message);
+                }
             }
         }
 
@@ -196,7 +204,10 @@ public bool ParseLibraries()
 
                 using var res = ClangParser.ParseLibrary(linkerOptions);
                 if (res.Kind != ParserResultKind.Success)
+                {
+                    res.Dispose();
                     continue;
+                }
 
                 for (uint i = 0; i < res.LibrariesCount; i++)
                     Context.Symbols.Libraries.Add(ClangParser.ConvertLibrary(res.GetLibraries(i)));
@@ -206,7 +217,7 @@ public bool ParseLibraries()
             Context.Symbols.IndexSymbols();
             SortModulesByDependencies();
 
-            return true;
+            return !hasParsingErrors;
         }
 
         public void SetupPasses(ILibrary library)
@@ -412,7 +423,7 @@ public void Dispose()
 
     public static class ConsoleDriver
     {
-        public static void Run(ILibrary library)
+        public static bool Run(ILibrary library)
         {
             var options = new DriverOptions();
             using var driver = new Driver(options);
@@ -427,15 +438,15 @@ public static void Run(ILibrary library)
                 Diagnostics.Message("Parsing libraries...");
 
             if (!driver.ParseLibraries())
-                return;
+                return false;
 
             if (!options.Quiet)
                 Diagnostics.Message("Parsing code...");
 
             if (!driver.ParseCode())
             {
                 Diagnostics.Error("CppSharp has encountered an error while parsing code.");
-                return;
+                return false;
             }
 
             new CleanUnitPass { Context = driver.Context }.VisitASTContext(driver.Context.ASTContext);
@@ -462,7 +473,7 @@ public static void Run(ILibrary library)
                 Diagnostics.Message("Generating code...");
 
             if (options.DryRun)
-                return;
+                return true;
 
             var outputs = driver.GenerateCode();
 
@@ -477,6 +488,8 @@ public static void Run(ILibrary library)
             driver.SaveCode(outputs);
             if (driver.Options.IsCSharpGenerator && driver.Options.CompileCode)
                 driver.Options.Modules.Any(m => !driver.CompileCode(m));
+
+            return true;
         }
     }
 }

src/Parser/ASTConverter.cs

@@ -847,7 +847,7 @@ public unsafe class DeclConverter : DeclVisitor<AST.Declaration>
         readonly CommentConverter commentConverter;
         readonly StmtConverter stmtConverter;
 
-        readonly Dictionary<IntPtr, AST.Declaration> Declarations;
+        readonly Dictionary<(IntPtr, DeclarationKind), AST.Declaration> Declarations;
         readonly Dictionary<IntPtr, AST.PreprocessedEntity> PreprocessedEntities;
         readonly Dictionary<IntPtr, AST.FunctionTemplateSpecialization> FunctionTemplateSpecializations;
 
@@ -857,7 +857,7 @@ public DeclConverter(TypeConverter type, CommentConverter comment, StmtConverter
             typeConverter = type;
             commentConverter = comment;
             stmtConverter = stmt;
-            Declarations = new Dictionary<IntPtr, AST.Declaration>();
+            Declarations = new Dictionary<(IntPtr, DeclarationKind), AST.Declaration>();
             PreprocessedEntities = new Dictionary<IntPtr, AST.PreprocessedEntity>();
             FunctionTemplateSpecializations = new Dictionary<IntPtr, AST.FunctionTemplateSpecialization>();
         }
@@ -876,8 +876,9 @@ public override AST.Declaration Visit(Declaration decl)
 
             // Check if the declaration was already handled and return its
             // existing instance.
-            if (CheckForDuplicates(decl) && Declarations.ContainsKey(originalPtr))
-                return Declarations[originalPtr];
+            var key = (decl.OriginalPtr, decl.Kind);
+            if (CheckForDuplicates(decl) && Declarations.TryGetValue(key, out var visit))
+                return visit;
 
             return base.Visit(decl);
         }
@@ -981,15 +982,17 @@ bool CheckForDuplicates(Declaration decl)
 
         void VisitDeclaration(Declaration decl, AST.Declaration _decl)
         {
-            var originalPtr = decl.OriginalPtr;
+            var key = (decl.OriginalPtr, decl.Kind);
 
             if (CheckForDuplicates(decl))
-                if (Declarations.ContainsKey(originalPtr))
+            {
+                if (Declarations.ContainsKey(key))
                     throw new NotSupportedException("Duplicate declaration processed");
+            }
 
             // Add the declaration to the map so that we can check if have
             // already handled it and return the declaration.
-            Declarations[originalPtr] = _decl;
+            Declarations[key] = _decl;
 
             _decl.Access = VisitAccessSpecifier(decl.Access);
             _decl.Name = decl.Name;
@@ -1020,7 +1023,7 @@ void VisitDeclaration(Declaration decl, AST.Declaration _decl)
                 _decl.PreprocessedEntities.Add(_entity);
             }
 
-            _decl.OriginalPtr = originalPtr;
+            _decl.OriginalPtr = decl.OriginalPtr;
 
             NativeObjects.Add(decl);