[Lldb-commits] [lldb] d48ef7c - [lldb] Print full Clang diagnostics when the ClangModulesDeclVendor fails to compile a module
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Fri May 15 01:11:39 PDT 2020
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 @@
+ at 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 * }
More information about the lldb-commits
mailing list