[lldb] Print full Clang diagnostics when the ClangModulesDeclVendor fails to compile a module

[teemperor]

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

[aprantl]

lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

392

"Unknown error while ..." anything more specific we can say will help figuring out at least *which step* failed.

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

 //===-- ClangModulesDeclVendor.cpp ----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <mutex>
 
 #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"
 #include "clang/Sema/Lookup.h"
 #include "clang/Serialization/ASTReader.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Threading.h"
public:
 
   void HandleDiagnostic(clang::DiagnosticsEngine::Level DiagLevel,
                         const clang::Diagnostic &info) override;
 
   void ClearDiagnostics();
 
   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 different diagnostics.
+  std::string m_output;
   Log *m_log;
 };
 
 // The private implementation of our ClangModulesDeclVendor.  Contains all the
 // Clang state required to load modules.
 class ClangModulesDeclVendorImpl : public ClangModulesDeclVendor {
 public:
   ClangModulesDeclVendorImpl(
private:
   // We assume that every ASTContext has an TypeSystemClang, so we also store
   // a custom TypeSystemClang for our internal ASTContext.
   std::unique_ptr<TypeSystemClang> m_ast_context;
 };
 } // anonymous namespace
 
 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(); }
 
 void StoringDiagnosticConsumer::DumpDiagnostics(Stream &error_stream) {
   for (IDAndDiagnostic &diag : m_diagnostics) {
     switch (diag.first) {
     default:
       error_stream.PutCString(diag.second);
       error_stream.PutChar('\n');
       break;
     case clang::DiagnosticsEngine::Level::Ignored:
       break;
     }
   }
 }
 
+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) {}
 
 ClangModulesDeclVendor::~ClangModulesDeclVendor() {}
 
 ClangModulesDeclVendorImpl::ClangModulesDeclVendorImpl(
     llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine> diagnostics_engine,
     std::shared_ptr<clang::CompilerInvocation> compiler_invocation,

lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

bool ClangUserExpression::SetupPersistentState(DiagnosticManager &diagnostic_manager,
   } else {
     diagnostic_manager.PutString(eDiagnosticSeverityError,
                                  "error: couldn't start parsing (no target)");
     return false;
   }
   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;
 
   if (!target->GetEnableAutoImportClangModules())
     return;
 
   auto *persistent_state = llvm::cast<ClangPersistentVariables>(
static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target,
   block->CalculateSymbolContext(&sc);
 
   if (!sc.comp_unit)
     return;
   StreamString error_stream;
 
   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() {
   m_expr_lang = lldb::LanguageType::eLanguageTypeUnknown;
   if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel)
     return;
   if (m_in_cplusplus_method)
     m_expr_lang = lldb::eLanguageTypeC_plus_plus;
bool ClangUserExpression::PrepareForParsing(
   }
 
   ////////////////////////////////////
   // Generate the expression
   //
 
   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 =
       module_config.GetImportedModules();
   m_imported_cpp_modules = !imported_modules.empty();
   m_include_directories = module_config.GetIncludeDirs();
 
   LLDB_LOG(log, "List of imported modules in expression: {0}",

lldb/test/API/lang/objc/modules-compile-error/Makefile

+OBJC_SOURCES := main.m
+
+CFLAGS_EXTRAS = $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(BUILDDIR) -DONLY_CLANG=1
+
+include Makefile.rules

lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py

+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'"])

lldb/test/API/lang/objc/modules-compile-error/main.m

+@import LLDBTestModule;
+
+int main() {
+  return foo(); // break here
+}

lldb/test/API/lang/objc/modules-compile-error/module.h

+int foo() { return 123; }
+
+#ifndef ONLY_CLANG
+syntax_error_for_lldb_to_find // comment that tests source printing
+#endif

lldb/test/API/lang/objc/modules-compile-error/module.modulemap

+module LLDBTestModule { header "module.h" export * }