Skip to content

Conversation

@Michael137
Copy link
Member

@Michael137 Michael137 commented Nov 7, 2025

Depends on:

While these errors can contribute to an expression failing, they are never the reason the expression failed. I.e., they are always just 'note:' diagnostics that we hand-emit. Because they are quite noisy (and we potentially have many of them if we auto-load all modules in a CU), this patch logs the errors to the expr log, instead of the console.

Previously these errors would only get omitted when the expression itself failed. Meaning if the expression failed, we'd dump these 'note' module load errors in next to the actual expression error, obscuring the output. Moreover, if the expression succeeded, any module load errors would be dropped. Now we always log all module loading errors to the expression log, regardless of whether the expression fails or not.

rdar://164002569

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Depends on:

While these errors can contribute to an expression failing, they are never the reason the expression failed. I.e., they are always just 'note:' diagnostics that we hand-emit. Because they are quite noisy (and we potentially have many of them if we auto-load all modules in a CU), this patch logs the errors to the expr log, instead of the console.

Previously these errors would only get omitted when the expression itself failed. Meaning if the expression failed, we'd dump these 'note' module load errors in next to the actual expression error, obscuring the output. Moreover, if the expression succeeded, any module load errors would be dropped. Now we always log all module loading errors to the expression log, regardless of whether the expression fails or not.


Patch is 28.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166964.diff

13 Files Affected:

  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+5-3)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp (+5-4)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (+56-46)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h (+4-14)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp (+7-15)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_FromExpression.test (+54)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidNestedSubmodule.test (+70)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test (+59)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test (+62)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test (+59)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test (+57)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test (+58)
  • (added) lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test (+53)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 6bab880b4d521..75ed87baf636a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -115,6 +115,7 @@ class ClangExpressionParser::LLDBPreprocessorCallbacks : public PPCallbacks {
   ClangModulesDeclVendor &m_decl_vendor;
   ClangPersistentVariables &m_persistent_vars;
   clang::SourceManager &m_source_mgr;
+  /// Accumulates error messages across all moduleImport calls.
   StreamString m_error_stream;
   bool m_has_errors = false;
 
@@ -140,11 +141,12 @@ class ClangExpressionParser::LLDBPreprocessorCallbacks : public PPCallbacks {
       module.path.push_back(
           ConstString(component.getIdentifierInfo()->getName()));
 
-    StreamString error_stream;
-
     ClangModulesDeclVendor::ModuleVector exported_modules;
-    if (!m_decl_vendor.AddModule(module, &exported_modules, m_error_stream))
+    if (auto err = m_decl_vendor.AddModule(module, &exported_modules)) {
       m_has_errors = true;
+      m_error_stream.PutCString(llvm::toString(std::move(err)));
+      m_error_stream.PutChar('\n');
+    }
 
     for (ClangModulesDeclVendor::ModuleID module : exported_modules)
       m_persistent_vars.AddHandLoadedClangModule(module);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index ff9ed9c27f70f..ad48d293ab8f0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -383,10 +383,11 @@ bool ClangExpressionSourceCode::GetText(
             block->CalculateSymbolContext(&sc);
 
             if (sc.comp_unit) {
-              StreamString error_stream;
-
-              decl_vendor->AddModulesForCompileUnit(
-                  *sc.comp_unit, modules_for_macros, error_stream);
+              if (auto err = decl_vendor->AddModulesForCompileUnit(
+                      *sc.comp_unit, modules_for_macros))
+                LLDB_LOG_ERROR(
+                    GetLog(LLDBLog::Expressions), std::move(err),
+                    "Error while loading hand-imported modules:\n{0}");
             }
           }
         }
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index b77e2690deb06..7635e49051216 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -92,11 +92,11 @@ class ClangModulesDeclVendorImpl : public ClangModulesDeclVendor {
 
   ~ClangModulesDeclVendorImpl() override = default;
 
-  bool AddModule(const SourceModule &module, ModuleVector *exported_modules,
-                 Stream &error_stream) override;
+  llvm::Error AddModule(const SourceModule &module,
+                        ModuleVector *exported_modules) override;
 
-  bool AddModulesForCompileUnit(CompileUnit &cu, ModuleVector &exported_modules,
-                                Stream &error_stream) override;
+  llvm::Error AddModulesForCompileUnit(CompileUnit &cu,
+                                       ModuleVector &exported_modules) override;
 
   uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
                      std::vector<CompilerDecl> &decls) override;
@@ -273,16 +273,14 @@ void ClangModulesDeclVendorImpl::ReportModuleExports(
     exports.push_back(module);
 }
 
-bool ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
-                                           ModuleVector *exported_modules,
-                                           Stream &error_stream) {
+llvm::Error
+ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
+                                      ModuleVector *exported_modules) {
   // Fail early.
 
-  if (m_compiler_instance->hadModuleLoaderFatalFailure()) {
-    error_stream.PutCString("error: Couldn't load a module because the module "
-                            "loader is in a fatal state.\n");
-    return false;
-  }
+  if (m_compiler_instance->hadModuleLoaderFatalFailure())
+    return llvm::createStringError(
+        "couldn't load a module because the module loader is in a fatal state");
 
   // Check if we've already imported this module.
 
@@ -297,7 +295,7 @@ bool ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
     if (mi != m_imported_modules.end()) {
       if (exported_modules)
         ReportModuleExports(*exported_modules, mi->second);
-      return true;
+      return llvm::Error::success();
     }
   }
 
@@ -315,30 +313,30 @@ bool ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
                             std::equal(sysroot_begin, sysroot_end, path_begin);
     // No need to inject search paths to modules in the sysroot.
     if (!is_system_module) {
-      auto error = [&]() {
-        error_stream.Printf("error: No module map file in %s\n",
-                            module.search_path.AsCString());
-        return false;
-      };
-
       bool is_system = true;
       bool is_framework = false;
       auto dir = HS.getFileMgr().getOptionalDirectoryRef(
           module.search_path.GetStringRef());
       if (!dir)
-        return error();
+        return llvm::createStringError(
+            "couldn't find module search path directory %s",
+            module.search_path.GetCString());
+
       auto file = HS.lookupModuleMapFile(*dir, is_framework);
       if (!file)
-        return error();
+        return llvm::createStringError("couldn't find modulemap file in %s",
+                                       module.search_path.GetCString());
+
       if (HS.parseAndLoadModuleMapFile(*file, is_system))
-        return error();
+        return llvm::createStringError(
+            "failed to parse and load modulemap file in %s",
+            module.search_path.GetCString());
     }
   }
-  if (!HS.lookupModule(module.path.front().GetStringRef())) {
-    error_stream.Printf("error: Header search couldn't locate module '%s'\n",
-                        module.path.front().AsCString());
-    return false;
-  }
+
+  if (!HS.lookupModule(module.path.front().GetStringRef()))
+    return llvm::createStringError("header search couldn't locate module '%s'",
+                                   module.path.front().AsCString());
 
   llvm::SmallVector<clang::IdentifierLoc, 4> clang_path;
 
@@ -364,22 +362,29 @@ bool ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
   clang::Module *top_level_module = DoGetModule(clang_path.front(), false);
 
   if (!top_level_module) {
+    lldb_private::StreamString error_stream;
     diagnostic_consumer->DumpDiagnostics(error_stream);
-    error_stream.Printf("error: Couldn't load top-level module %s\n",
-                        module.path.front().AsCString());
-    return false;
+
+    return llvm::createStringError(llvm::formatv(
+        "couldn't load top-level module {0}:\n{1}",
+        module.path.front().GetStringRef(), error_stream.GetString()));
   }
 
   clang::Module *submodule = top_level_module;
 
   for (auto &component : llvm::ArrayRef<ConstString>(module.path).drop_front()) {
-    submodule = submodule->findSubmodule(component.GetStringRef());
-    if (!submodule) {
+    clang::Module *found = submodule->findSubmodule(component.GetStringRef());
+    if (!found) {
+      lldb_private::StreamString error_stream;
       diagnostic_consumer->DumpDiagnostics(error_stream);
-      error_stream.Printf("error: Couldn't load submodule %s\n",
-                          component.GetCString());
-      return false;
+
+      return llvm::createStringError(llvm::formatv(
+          "couldn't load submodule '{0}' of module '{1}':\n{2}",
+          component.GetStringRef(), submodule->getFullModuleName(),
+          error_stream.GetString()));
     }
+
+    submodule = found;
   }
 
   // If we didn't make the submodule visible here, Clang wouldn't allow LLDB to
@@ -399,10 +404,12 @@ bool ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
 
     m_enabled = true;
 
-    return true;
+    return llvm::Error::success();
   }
 
-  return false;
+  return llvm::createStringError(
+      llvm::formatv("unknown error while loading module {0}\n",
+                    module.path.front().GetStringRef()));
 }
 
 bool ClangModulesDeclVendor::LanguageSupportsClangModules(
@@ -424,15 +431,18 @@ bool ClangModulesDeclVendor::LanguageSupportsClangModules(
   }
 }
 
-bool ClangModulesDeclVendorImpl::AddModulesForCompileUnit(
-    CompileUnit &cu, ClangModulesDeclVendor::ModuleVector &exported_modules,
-    Stream &error_stream) {
-  if (LanguageSupportsClangModules(cu.GetLanguage())) {
-    for (auto &imported_module : cu.GetImportedModules())
-      if (!AddModule(imported_module, &exported_modules, error_stream))
-        return false;
-  }
-  return true;
+llvm::Error ClangModulesDeclVendorImpl::AddModulesForCompileUnit(
+    CompileUnit &cu, ClangModulesDeclVendor::ModuleVector &exported_modules) {
+  if (!LanguageSupportsClangModules(cu.GetLanguage()))
+    return llvm::Error::success();
+
+  llvm::Error errors = llvm::Error::success();
+
+  for (auto &imported_module : cu.GetImportedModules())
+    if (auto err = AddModule(imported_module, &exported_modules))
+      errors = llvm::joinErrors(std::move(errors), std::move(err));
+
+  return errors;
 }
 
 // ClangImporter::lookupValue
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h
index debf4761175b8..043632007b7d3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h
@@ -45,17 +45,12 @@ class ClangModulesDeclVendor : public DeclVendor {
   ///     If non-NULL, a pointer to a vector to populate with the ID of every
   ///     module that is re-exported by the specified module.
   ///
-  /// \param[out] error_stream
-  ///     A stream to populate with the output of the Clang parser when
-  ///     it tries to load the module.
-  ///
   /// \return
   ///     True if the module could be loaded; false if not.  If the
   ///     compiler encountered a fatal error during a previous module
   ///     load, then this will always return false for this ModuleImporter.
-  virtual bool AddModule(const SourceModule &module,
-                         ModuleVector *exported_modules,
-                         Stream &error_stream) = 0;
+  virtual llvm::Error AddModule(const SourceModule &module,
+                                ModuleVector *exported_modules) = 0;
 
   /// Add all modules referred to in a given compilation unit to the list
   /// of modules to search.
@@ -67,18 +62,13 @@ class ClangModulesDeclVendor : public DeclVendor {
   ///     A vector to populate with the ID of each module loaded (directly
   ///     and via re-exports) in this way.
   ///
-  /// \param[out] error_stream
-  ///     A stream to populate with the output of the Clang parser when
-  ///     it tries to load the modules.
-  ///
   /// \return
   ///     True if all modules referred to by the compilation unit could be
   ///     loaded; false if one could not be loaded.  If the compiler
   ///     encountered a fatal error during a previous module
   ///     load, then this will always return false for this ModuleImporter.
-  virtual bool AddModulesForCompileUnit(CompileUnit &cu,
-                                        ModuleVector &exported_modules,
-                                        Stream &error_stream) = 0;
+  virtual llvm::Error
+  AddModulesForCompileUnit(CompileUnit &cu, ModuleVector &exported_modules) = 0;
 
   /// Enumerate all the macros that are defined by a given set of modules
   /// that are already imported.
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index e8d5ec3c7fd96..59135964b8c8c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -371,26 +371,18 @@ static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target,
 
   if (!sc.comp_unit)
     return;
-  StreamString error_stream;
-
   ClangModulesDeclVendor::ModuleVector modules_for_macros =
       persistent_state->GetHandLoadedClangModules();
-  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(lldb::eSeverityInfo, error_stream.GetString());
+  auto err =
+      decl_vendor->AddModulesForCompileUnit(*sc.comp_unit, modules_for_macros);
+  if (!err)
     return;
-  }
 
-  diagnostic_manager.PutString(lldb::eSeverityError,
-                               "Unknown error while loading modules needed for "
-                               "current compilation unit.");
+  llvm::handleAllErrors(
+      std::move(err), [](const llvm::StringError &e) {
+        LLDB_LOG(GetLog(LLDBLog::Expressions), "{0}", e.getMessage());
+      });
 }
 
 ClangExpressionSourceCode::WrapKind ClangUserExpression::GetWrapKind() const {
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_FromExpression.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_FromExpression.test
new file mode 100644
index 0000000000000..b964e9b27e914
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_FromExpression.test
@@ -0,0 +1,54 @@
+## Tests the case where we fail to import modules from @import
+## statements that are part of the expression being run.
+#
+# REQUIRES: system-darwin
+#
+# RUN: split-file %s %t/sources
+# RUN: %clang_host -g %t/sources/main.m -fmodules -fcxx-modules \
+# RUN:             -fmodule-map-file=%t/sources/module.modulemap \
+# RUN:             -fmodules-cache-path=%t/ModuleCache -o %t.out
+#
+# RUN: sed -i '' -e 's/foo\.h/baz\.h/' %t/sources/module.modulemap
+#
+# RUN: %lldb -x -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:       -s %t/sources/commands.input %t.out -o exit 2>&1 | FileCheck %s
+
+#--- main.m
+@import foo;
+@import bar;
+
+int main() { __builtin_debugtrap(); }
+
+#--- foo.h
+struct foo {};
+
+#--- bar.h
+struct bar {};
+
+#--- module.modulemap
+module foo {
+    header "foo.h"
+    export *
+}
+
+module bar {
+    header "bar.h"
+    export *
+}
+
+#--- commands.input
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr @import Foo; @import Bar
+expr @import foo
+
+# CHECK: error: while importing modules:
+# CHECK-NEXT: header search couldn't locate module 'Foo'
+# CHECK-NEXT: header search couldn't locate module 'Bar'
+#
+# CHECK: expr @import foo
+# CHECK: error: while importing modules:
+# CHECK-NEXT: couldn't load top-level module foo
+## No mention of the previous import errors.
+# CHECK-NOT: Foo
+# CHECK-NOT: Bar
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidNestedSubmodule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidNestedSubmodule.test
new file mode 100644
index 0000000000000..1e8075dd20fad
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidNestedSubmodule.test
@@ -0,0 +1,70 @@
+## Tests the case where we fail to load a submodule of a submodule. We force this
+## by removing the submodule 'module qux' of 'module baz' from the modulemap.
+#
+# REQUIRES: system-darwin
+#
+# RUN: split-file %s %t/sources
+# RUN: %clang_host -g %t/sources/main.m -fmodules -fcxx-modules \
+# RUN:             -fmodule-map-file=%t/sources/module.modulemap \
+# RUN:             -fmodules-cache-path=%t/ModuleCache -o %t.out
+# RUN: sed -i '' -e 's/module qux/module quz/' %t/sources/module.modulemap
+#
+# RUN: %lldb -x -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:       -s %t/sources/commands.input %t.out -o exit 2>&1 | FileCheck %s --check-prefix=NO_LOG
+#
+# RUN: %lldb -x -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:       -s %t/sources/commands-with-log.input %t.out -o exit 2>&1 | FileCheck %s --check-prefix=LOG
+
+#--- main.m
+@import foo.baz.qux;
+@import bar;
+
+int main() { __builtin_debugtrap(); }
+
+#--- foo.h
+struct foo {};
+
+#--- bar.h
+struct bar {};
+
+#--- baz.h
+struct baz {};
+
+#--- qux.h
+struct qux {};
+
+#--- module.modulemap
+module foo {
+    header "foo.h"
+    export *
+
+    module baz {
+        header "baz.h"
+        export *
+
+        module qux {
+            header "qux.h"
+            export *
+        }
+    }
+}
+
+module bar {
+    header "bar.h"
+    export *
+}
+
+#--- commands.input
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr blah
+
+# NO_LOG-NOT: couldn't load submodule 'qux' of module 'foo.baz'
+
+#--- commands-with-log.input
+log enable lldb expr
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr blah
+
+# LOG: couldn't load submodule 'qux' of module 'foo.baz'
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
new file mode 100644
index 0000000000000..35ba5802d2add
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
@@ -0,0 +1,59 @@
+## Tests the case where the DW_AT_LLVM_include_path of the module is invalid.
+## We forces this by just removing that directory (which in our case is 'sources').
+#
+# REQUIRES: system-darwin
+#
+# RUN: split-file %s %t/sources
+# RUN: %clang_host -g %t/sources/main.m -fmodules -fcxx-modules \
+# RUN:             -fmodule-map-file=%t/sources/module.modulemap \
+# RUN:             -fmodules-cache-path=%t/ModuleCache -o %t.out
+#
+# RUN: cp %t/sources/commands.input %t/commands.input
+# RUN: cp %t/sources/commands-with-log.input %t/commands-with-log.input
+# RUN: rm -r %t/sources
+#
+# RUN: %lldb -x -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:       -s %t/commands.input %t.out -o exit 2>&1 | FileCheck %s --check-prefix=NO_LOG
+#
+# RUN: %lldb -x -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:       -s %t/commands-with-log.input %t.out -o exit 2>&1 | FileCheck %s --check-prefix=LOG
+
+#--- main.m
+@import foo;
+@import bar;
+
+int main() { __builtin_debugtrap(); }
+
+#--- foo.h
+struct foo {};
+
+#--- bar.h
+struct bar {};
+
+#--- module.modulemap
+module foo {
+    header "foo.h"
+    export *
+}
+
+module bar {
+    header "bar.h"
+    export *
+}
+
+#--- commands.input
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr blah
+
+# NO_LOG-NOT: couldn't find module search path directory {{.*}}sources
+# NO_LOG-NOT: couldn't find module search path directory {{.*}}sources
+
+#--- commands-with-log.input
+log enable lldb expr
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr blah
+
+# LOG: couldn't find module search path directory {{.*}}sources
+# LOG: couldn't find module search path directory {{.*}}sources
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test
new file mode 100644
index 0000000000000..1bfbbcf32ecae
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test
@@ -0,0 +1,62 @@
+## Tests the case where we fail to load a submodule. We force this by removing
+## the submodule 'module baz' from the modulemap.
+#
+# REQUIRES: system-darwin
+#
+# RUN: split-file %s %t/sources
+# RUN: %clang_host -g %t/sources/main.m -fmodules -fcxx-modules \
+# RUN:             -fmodule-map-file=%t/sources/module.modulemap \
+# RUN:             -fmodules-cache-path=%t/ModuleCache -o %t.out
+# RUN: sed -i '' -...
[truncated]

@github-actions
Copy link

github-actions bot commented Nov 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

*sc.comp_unit, modules_for_macros, error_stream);
if (auto err = decl_vendor->AddModulesForCompileUnit(
*sc.comp_unit, modules_for_macros))
LLDB_LOG_ERROR(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment here to explain that this isn't fatal and confusing for users, hence being printed to the log?

…xpression log instead of console

Depends on:
* llvm#166917
* llvm#166940

While these errors can contribute to an expression failing, they are never *the* reason the expression failed. I.e., they are always just 'note:' diagnostics that we hand-emit. Because they are quite noisy (and we potentially have many of them if we auto-load all modules in a CU), this patch logs the errors to the `expr` log, instead of the console.

Previously these errors would only get omitted when the expression itself failed. Meaning if the expression failed, we'd dump these 'note' module load errors in next to the actual expression error, obscuring the output. Moreover, if the expression succeeded, any module load errors would be dropped. Now we always log all module loading errors to the expression log, regardless of whether the expression fails or not.
@Michael137 Michael137 force-pushed the lldb/modules-loading-error-log branch from 3157cbe to 797a965 Compare November 8, 2025 00:06
@Michael137 Michael137 enabled auto-merge (squash) November 8, 2025 00:07
@Michael137 Michael137 merged commit d5b62fa into llvm:main Nov 8, 2025
8 of 9 checks passed
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Nov 8, 2025
…xpression log instead of console (llvm#166964)

Depends on:
* llvm#166917
* llvm#166940

While these errors can contribute to an expression failing, they are
never *the* reason the expression failed. I.e., they are always just
'note:' diagnostics that we hand-emit. Because they are quite noisy (and
we potentially have many of them if we auto-load all modules in a CU),
this patch logs the errors to the `expr` log, instead of the console.

Previously these errors would only get omitted when the expression
itself failed. Meaning if the expression failed, we'd dump these 'note'
module load errors in next to the actual expression error, obscuring the
output. Moreover, if the expression succeeded, any module load errors
would be dropped. Now we always log all module loading errors to the
expression log, regardless of whether the expression fails or not.

(cherry picked from commit d5b62fa)
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 8, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building lldb at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/26998

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: commands/log/basic/TestLogHandlers.py (188 of 2370)
PASS: lldb-api :: commands/log/invalid-args/TestInvalidArgsLog.py (189 of 2370)
PASS: lldb-api :: commands/help/TestHelp.py (190 of 2370)
PASS: lldb-api :: commands/frame/var/TestFrameVar.py (191 of 2370)
PASS: lldb-api :: commands/platform/basic/TestPlatformPython.py (192 of 2370)
PASS: lldb-api :: commands/platform/basic/TestPlatformCommand.py (193 of 2370)
PASS: lldb-api :: commands/memory/write/TestMemoryWrite.py (194 of 2370)
PASS: lldb-api :: commands/platform/file/close/TestPlatformFileClose.py (195 of 2370)
PASS: lldb-api :: commands/platform/file/read/TestPlatformFileRead.py (196 of 2370)
UNRESOLVED: lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py (197 of 2370)
******************** TEST 'lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --cmake-build-type Release /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/commands/gui/spawn-threads -p TestGuiSpawnThreads.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision d5b62fabd17ee14805f4c5c5ac7b0be9bf488211)
  clang revision d5b62fabd17ee14805f4c5c5ac7b0be9bf488211
  llvm revision d5b62fabd17ee14805f4c5c5ac7b0be9bf488211
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'pdb', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
FAIL: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_gui (TestGuiSpawnThreads.TestGuiSpawnThreadsTest)
======================================================================
ERROR: test_gui (TestGuiSpawnThreads.TestGuiSpawnThreadsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 156, in wrapper
    return func(*args, **kwargs)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py", line 44, in test_gui
    self.child.expect_exact(f"thread #{i + 2}: tid =")
  File "/usr/local/lib/python3.10/dist-packages/pexpect/spawnbase.py", line 432, in expect_exact
    return exp.expect_loop(timeout)
  File "/usr/local/lib/python3.10/dist-packages/pexpect/expect.py", line 179, in expect_loop
    return self.eof(e)
  File "/usr/local/lib/python3.10/dist-packages/pexpect/expect.py", line 122, in eof
    raise exc
pexpect.exceptions.EOF: End Of File (EOF). Exception style platform.
<pexpect.pty_spawn.spawn object at 0xfde1306ad420>
command: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb
args: ['/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb', '--no-lldbinit', '--no-use-colors', '-O', 'settings clear --all', '-O', 'settings set symbols.enable-external-lookup false', '-O', 'settings set target.inherit-tcc true', '-O', 'settings set target.disable-aslr false', '-O', 'settings set target.detach-on-error false', '-O', 'settings set target.auto-apply-fixits false', '-O', 'settings set plugin.process.gdb-remote.packet-timeout 60', '-O', 'settings set symbols.clang-modules-cache-path "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api"', '-O', 'settings set use-color false', '-O', 'settings set show-statusline false', '--file', '/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/commands/gui/spawn-threads/TestGuiSpawnThreads.test_gui/a.out']
buffer (last 100 chars): b''
before (last 100 chars): b'2 0x0000bada478d5330 _start (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb+0x45330)\n'
after: <class 'pexpect.exceptions.EOF'>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants