From e464da48c80013969302c2757d63998340d4b0c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Matos?= Date: Thu, 28 Sep 2023 18:06:33 +0100 Subject: [PATCH] Bug fixes for GCC 11, cache conversion robustness and error handling (#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. --- src/CppParser/AST.cpp | 2 ++ src/CppParser/AST.h | 1 + src/CppParser/CppParser.h | 6 +++--- src/CppParser/Parser.cpp | 8 +++++++- src/Generator/Driver.cs | 29 +++++++++++++++++++++-------- src/Parser/ASTConverter.cs | 19 +++++++++++-------- 6 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/CppParser/AST.cpp b/src/CppParser/AST.cpp index ae9e176360..2e67759824 100644 --- a/src/CppParser/AST.cpp +++ b/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) diff --git a/src/CppParser/AST.h b/src/CppParser/AST.h index ceb1064f60..c8a3be1cb8 100644 --- a/src/CppParser/AST.h +++ b/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; }; diff --git a/src/CppParser/CppParser.h b/src/CppParser/CppParser.h index 922cabfb23..f0e479cfd9 100644 --- a/src/CppParser/CppParser.h +++ b/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 diff --git a/src/CppParser/Parser.cpp b/src/CppParser/Parser.cpp index 97d7736643..4e55c574db 100644 --- a/src/CppParser/Parser.cpp +++ b/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; } diff --git a/src/Generator/Driver.cs b/src/Generator/Driver.cs index 82780bfa2c..86156c93c5 100644 --- a/src/Generator/Driver.cs +++ b/src/Generator/Driver.cs @@ -137,9 +137,17 @@ void OnFileParsed(IEnumerable 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,7 +438,7 @@ public static void Run(ILibrary library) Diagnostics.Message("Parsing libraries..."); if (!driver.ParseLibraries()) - return; + return false; if (!options.Quiet) Diagnostics.Message("Parsing code..."); @@ -435,7 +446,7 @@ public static void Run(ILibrary library) 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; } } } diff --git a/src/Parser/ASTConverter.cs b/src/Parser/ASTConverter.cs index f32f63c75b..8c4a82fb04 100644 --- a/src/Parser/ASTConverter.cs +++ b/src/Parser/ASTConverter.cs @@ -847,7 +847,7 @@ public unsafe class DeclConverter : DeclVisitor readonly CommentConverter commentConverter; readonly StmtConverter stmtConverter; - readonly Dictionary Declarations; + readonly Dictionary<(IntPtr, DeclarationKind), AST.Declaration> Declarations; readonly Dictionary PreprocessedEntities; readonly Dictionary FunctionTemplateSpecializations; @@ -857,7 +857,7 @@ public DeclConverter(TypeConverter type, CommentConverter comment, StmtConverter typeConverter = type; commentConverter = comment; stmtConverter = stmt; - Declarations = new Dictionary(); + Declarations = new Dictionary<(IntPtr, DeclarationKind), AST.Declaration>(); PreprocessedEntities = new Dictionary(); FunctionTemplateSpecializations = new Dictionary(); } @@ -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);