Author: Raphael Isemann Date: 2020-05-15T10:11:03+02:00 New Revision: d48ef7cab55878fbb598e7a968b6073f9c7aa9ed
URL: https://github.com/llvm/llvm-project/commit/d48ef7cab55878fbb598e7a968b6073f9c7aa9ed DIFF: https://github.com/llvm/llvm-project/commit/d48ef7cab55878fbb598e7a968b6073f9c7aa9ed.diff LOG: [lldb] Print full Clang diagnostics when the ClangModulesDeclVendor fails to compile a module Summary: When the ClangModulesDeclVendor currently fails it just prints very basic and often incomplete diagnostics without any source locations: ``` (lldb) p @import Foundation error: while importing modules: 'foo/bar.h' file not found could not build module 'Darwin' [...] ``` or even just ``` (lldb) p @import Foundation error: while importing modules: could not build module 'Darwin' [...] ``` These diagnostics help neither the user nor us with figuring out what is the reason for the failure. This patch wires up a full TextDiagnosticPrinter in the ClangModulesDeclVendor and makes sure we always return the error stream to the user when we fail to compile our modules. Fixes rdar://63216849 Reviewers: aprantl, jdoerfert Reviewed By: aprantl Subscribers: JDevlieghere Differential Revision: https://reviews.llvm.org/D79947 Added: lldb/test/API/lang/objc/modules-compile-error/Makefile lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py lldb/test/API/lang/objc/modules-compile-error/main.m lldb/test/API/lang/objc/modules-compile-error/module.h lldb/test/API/lang/objc/modules-compile-error/module.modulemap Modified: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp index 4b0521aff411..e003de519884 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp @@ -11,6 +11,7 @@ #include "clang/Basic/TargetInfo.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" +#include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" #include "clang/Parse/Parser.h" @@ -54,10 +55,21 @@ class StoringDiagnosticConsumer : public clang::DiagnosticConsumer { void DumpDiagnostics(Stream &error_stream); + void BeginSourceFile(const clang::LangOptions &LangOpts, + const clang::Preprocessor *PP = nullptr) override; + void EndSourceFile() override; + private: typedef std::pair<clang::DiagnosticsEngine::Level, std::string> IDAndDiagnostic; std::vector<IDAndDiagnostic> m_diagnostics; + /// The DiagnosticPrinter used for creating the full diagnostic messages + /// that are stored in m_diagnostics. + std::shared_ptr<clang::TextDiagnosticPrinter> m_diag_printer; + /// Output stream of m_diag_printer. + std::shared_ptr<llvm::raw_string_ostream> m_os; + /// Output string filled by m_os. Will be reused for diff erent diagnostics. + std::string m_output; Log *m_log; }; @@ -116,17 +128,21 @@ class ClangModulesDeclVendorImpl : public ClangModulesDeclVendor { StoringDiagnosticConsumer::StoringDiagnosticConsumer() { m_log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS); + + clang::DiagnosticOptions *m_options = new clang::DiagnosticOptions(); + m_os.reset(new llvm::raw_string_ostream(m_output)); + m_diag_printer.reset(new clang::TextDiagnosticPrinter(*m_os, m_options)); } void StoringDiagnosticConsumer::HandleDiagnostic( clang::DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &info) { - llvm::SmallVector<char, 256> diagnostic_string; + // Print the diagnostic to m_output. + m_output.clear(); + m_diag_printer->HandleDiagnostic(DiagLevel, info); + m_os->flush(); - info.FormatDiagnostic(diagnostic_string); - - m_diagnostics.push_back( - IDAndDiagnostic(DiagLevel, std::string(diagnostic_string.data(), - diagnostic_string.size()))); + // Store the diagnostic for later. + m_diagnostics.push_back(IDAndDiagnostic(DiagLevel, m_output)); } void StoringDiagnosticConsumer::ClearDiagnostics() { m_diagnostics.clear(); } @@ -144,6 +160,15 @@ void StoringDiagnosticConsumer::DumpDiagnostics(Stream &error_stream) { } } +void StoringDiagnosticConsumer::BeginSourceFile( + const clang::LangOptions &LangOpts, const clang::Preprocessor *PP) { + m_diag_printer->BeginSourceFile(LangOpts, PP); +} + +void StoringDiagnosticConsumer::EndSourceFile() { + m_diag_printer->EndSourceFile(); +} + ClangModulesDeclVendor::ClangModulesDeclVendor() : ClangDeclVendor(eClangModuleDeclVendor) {} diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index b2c6cba3bd58..d649f226b6b8 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -347,7 +347,8 @@ bool ClangUserExpression::SetupPersistentState(DiagnosticManager &diagnostic_man return true; } -static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target) { +static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target, + DiagnosticManager &diagnostic_manager) { ClangModulesDeclVendor *decl_vendor = target->GetClangModulesDeclVendor(); if (!decl_vendor) return; @@ -377,8 +378,23 @@ static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target) { ClangModulesDeclVendor::ModuleVector modules_for_macros = persistent_state->GetHandLoadedClangModules(); - decl_vendor->AddModulesForCompileUnit(*sc.comp_unit, modules_for_macros, - error_stream); + if (decl_vendor->AddModulesForCompileUnit(*sc.comp_unit, modules_for_macros, + error_stream)) + return; + + // Failed to load some modules, so emit the error stream as a diagnostic. + if (!error_stream.Empty()) { + // The error stream already contains several Clang diagnostics that might + // be either errors or warnings, so just print them all as one remark + // diagnostic to prevent that the message starts with "error: error:". + diagnostic_manager.PutString(eDiagnosticSeverityRemark, + error_stream.GetString()); + return; + } + + diagnostic_manager.PutString(eDiagnosticSeverityError, + "Unknown error while loading modules needed for " + "current compilation unit."); } void ClangUserExpression::UpdateLanguageForExpr() { @@ -530,7 +546,7 @@ bool ClangUserExpression::PrepareForParsing( ApplyObjcCastHack(m_expr_text); - SetupDeclVendor(exe_ctx, m_target); + SetupDeclVendor(exe_ctx, m_target, diagnostic_manager); CppModuleConfiguration module_config = GetModuleConfig(m_language, exe_ctx); llvm::ArrayRef<std::string> imported_modules = diff --git a/lldb/test/API/lang/objc/modules-compile-error/Makefile b/lldb/test/API/lang/objc/modules-compile-error/Makefile new file mode 100644 index 000000000000..e031aa0bbbb8 --- /dev/null +++ b/lldb/test/API/lang/objc/modules-compile-error/Makefile @@ -0,0 +1,5 @@ +OBJC_SOURCES := main.m + +CFLAGS_EXTRAS = $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(BUILDDIR) -DONLY_CLANG=1 + +include Makefile.rules diff --git a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py new file mode 100644 index 000000000000..a422e8d77550 --- /dev/null +++ b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py @@ -0,0 +1,23 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + @skipUnlessDarwin + def test(self): + self.build() + lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.m")) + + # Try importing our custom module. This will fail as LLDB won't define + # the CLANG_ONLY define when it compiles the module for the expression + # evaluator. + # Check that the error message shows file/line/column, prints the relevant + # line from the source code and mentions the module that failed to build. + self.expect("expr @import LLDBTestModule", error=True, + substrs=["module.h:4:1: error: unknown type name 'syntax_error_for_lldb_to_find'", + "syntax_error_for_lldb_to_find // comment that tests source printing", + "could not build module 'LLDBTestModule'"]) diff --git a/lldb/test/API/lang/objc/modules-compile-error/main.m b/lldb/test/API/lang/objc/modules-compile-error/main.m new file mode 100644 index 000000000000..35259dd287b0 --- /dev/null +++ b/lldb/test/API/lang/objc/modules-compile-error/main.m @@ -0,0 +1,5 @@ +@import LLDBTestModule; + +int main() { + return foo(); // break here +} diff --git a/lldb/test/API/lang/objc/modules-compile-error/module.h b/lldb/test/API/lang/objc/modules-compile-error/module.h new file mode 100644 index 000000000000..2edd13b0832d --- /dev/null +++ b/lldb/test/API/lang/objc/modules-compile-error/module.h @@ -0,0 +1,5 @@ +int foo() { return 123; } + +#ifndef ONLY_CLANG +syntax_error_for_lldb_to_find // comment that tests source printing +#endif diff --git a/lldb/test/API/lang/objc/modules-compile-error/module.modulemap b/lldb/test/API/lang/objc/modules-compile-error/module.modulemap new file mode 100644 index 000000000000..3d44faf3e908 --- /dev/null +++ b/lldb/test/API/lang/objc/modules-compile-error/module.modulemap @@ -0,0 +1 @@ +module LLDBTestModule { header "module.h" export * } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
