[Lldb-commits] [lldb] [lldb][ClangModulesDeclVendor] Don't stop loading Clang modules if an individual import failed (PR #166940)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Fri Nov 7 06:00:24 PST 2025
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/166940
Depends on:
* https://github.com/llvm/llvm-project/pull/166917
When loading all Clang modules for a CU, we stop on first error. This
means benign module loading errors may stop us from importing actually
useful modules. There's no good reason to bail on the first one. The
pathological would be if we try to load a large number of Clang modules
but all fail to load for the same reason. That could happen, but in
practice I've always seen only a handful of modules failing to load out
of a large number. Particularly system modules are useful and usually
don't fail to load. Whereas project-specific Clang modules are more
likely to fail because the build system moves the modulemap/sources
around.
This patch accumulates all module loading errors and doesn't stop when
an error is encountered.
>From 68cedd0963f85821f70f0abd2d80a9b67e13ffb4 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 6 Nov 2025 16:20:30 +0000
Subject: [PATCH 1/2] [lldb][ClangModulesDeclVendor] Revamp error handling when
loading Clang modules
Instead of propagating the errors as a `bool`+`Stream` we change the `ClangModulesDeclVendor` module loading APIs to use `llvm::Error`. We also reword some of the diagnostics (notably removing the hardcoded `error:` prefix). A follow-up patch will further make the module loading errors less noisy.
See the new tests for what the errors look like.
rdar://164002569
---
.../Clang/ClangExpressionParser.cpp | 8 +-
.../Clang/ClangExpressionSourceCode.cpp | 9 +-
.../Clang/ClangModulesDeclVendor.cpp | 102 ++++++++++--------
.../Clang/ClangModulesDeclVendor.h | 18 +---
.../Clang/ClangUserExpression.cpp | 22 ++--
...stClangModuleLoadError_FromExpression.test | 54 ++++++++++
...oduleLoadError_InvalidNestedSubmodule.test | 59 ++++++++++
...langModuleLoadError_InvalidSearchPath.test | 47 ++++++++
...ClangModuleLoadError_InvalidSubmodule.test | 51 +++++++++
...ModuleLoadError_InvalidTopLevelModule.test | 48 +++++++++
...ClangModuleLoadError_ModulemapParsing.test | 46 ++++++++
.../TestClangModuleLoadError_NoModule.test | 47 ++++++++
.../TestClangModuleLoadError_NoModuleMap.test | 42 ++++++++
13 files changed, 471 insertions(+), 82 deletions(-)
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_FromExpression.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidNestedSubmodule.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test
create mode 100644 lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test
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..b4e81aa21c138 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: {0}");
}
}
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index b77e2690deb06..6c5243b853ddf 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();
+
+ for (auto &imported_module : cu.GetImportedModules())
+ // TODO: don't short-circuit. Continue loading modules even if one of them
+ // fails. Concatenate all the errors.
+ if (auto err = AddModule(imported_module, &exported_modules))
+ return err;
+
+ return llvm::Error::success();
}
// 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..13d32b3bbc4f3 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.");
+ // FIXME: should we be dumping these to the error log instead of as
+ // diagnostics? They are non-fatal and are usually quite noisy.
+ diagnostic_manager.PutString(lldb::eSeverityInfo,
+ llvm::toString(std::move(err)));
}
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
+ at import foo;
+ at 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..a5822ae4a75e7
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidNestedSubmodule.test
@@ -0,0 +1,59 @@
+## 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
+
+#--- main.m
+ at import foo.baz.qux;
+ at 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
+
+# CHECK: note: 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..0fda05283608e
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
@@ -0,0 +1,47 @@
+## 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: 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
+
+#--- main.m
+ at import foo;
+ at 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
+
+# CHECK: note: couldn't find module search path directory {{.*}}sources
+## FIXME: We never attempted to load bar.
+# CHECK-NOT: couldn't find module search path
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test
new file mode 100644
index 0000000000000..2552cdb4b61db
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSubmodule.test
@@ -0,0 +1,51 @@
+## 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 '' -e 's/module baz/module qux/' %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
+ at import foo.baz;
+ at import bar;
+
+int main() { __builtin_debugtrap(); }
+
+#--- foo.h
+struct foo {};
+
+#--- bar.h
+struct bar {};
+
+#--- baz.h
+struct baz {};
+
+#--- module.modulemap
+module foo {
+ header "foo.h"
+ export *
+
+ module baz {
+ header "baz.h"
+ export *
+ }
+}
+
+module bar {
+ header "bar.h"
+ export *
+}
+
+#--- commands.input
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr blah
+
+# CHECK: note: couldn't load submodule 'baz' of module 'foo'
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test
new file mode 100644
index 0000000000000..a50f784dd1dc2
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test
@@ -0,0 +1,48 @@
+## Tests the case where a module fails to load. We force this by
+## replacing the contents of the 'module foo' declaration with garbage.
+#
+# 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: sed -i '' -e 's/bar\.h/qux\.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
+ at import foo;
+ at 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
+
+# CHECK: note: couldn't load top-level module foo
+## FIXME: clang error diagnostic shouldn't be dumped to the console.
+# CHECK: error:
+## FIXME: We never attempted to load bar.
+# CHECK-NOT: bar
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test
new file mode 100644
index 0000000000000..cadeb2de02c87
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test
@@ -0,0 +1,46 @@
+## Tests the case where the modulemap is semantically invalid and thus
+## Clang fails to load it on behalf of LLDB. We force this error by
+## creating a redefinition of 'module bar'.
+#
+# 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 foo/module bar/' %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
+ at import foo;
+ at 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
+
+# CHECK: note: failed to parse and load modulemap file in {{.*}}sources
+## FIXME: We never attempted to load bar.
+# CHECK-NOT: failed to parse and load modulemap
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test
new file mode 100644
index 0000000000000..a14f51eeba073
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test
@@ -0,0 +1,47 @@
+## Tests the case where the module LLDB is trying to load isn't
+## present in the modulemap. We force this by replacing 'module foo'
+## in 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 foo/module baz/' %t/sources/module.modulemap
+# RUN: sed -i '' -e 's/module bar/module qux/' %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
+ at import foo;
+ at 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
+
+# CHECK: note: header search couldn't locate module 'foo'
+## FIXME: We never attempted to load bar.
+# CHECK-NOT: bar
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test
new file mode 100644
index 0000000000000..a4fcace4f5b51
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test
@@ -0,0 +1,42 @@
+# 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: rm %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
+ at import foo;
+ at 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
+
+# CHECK: note: couldn't find modulemap file in {{.*}}sources
+## FIXME: We never attempted to load bar.
+# CHECK-NOT: couldn't find modulemap
>From d53acba090654316d3679b18a799f5135ecf3ace Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 7 Nov 2025 13:53:51 +0000
Subject: [PATCH 2/2] [lldb][ClangModulesDeclVendor] Don't stop loading Clang
modules if an individual import failed
When loading all Clang modules for a CU, we stop on first error. This
means benign module loading errors may stop us from importing actually
useful modules. There's no good reason to bail on the first one. The
pathological would be if we try to load a large number of Clang modules
but all fail to load for the same reason. That could happen, but in
practice I've always seen only a handful of modules failing to load out
of a large number. Particularly system modules are useful and usually
don't fail to load. Whereas project-specific Clang modules are more
likely to fail because the build system moves the modulemap/sources
around.
This patch accumulates all module loading errors and doesn't stop when
an error is encountered.
Depends on:
* https://github.com/llvm/llvm-project/pull/166917
---
.../ExpressionParser/Clang/ClangModulesDeclVendor.cpp | 8 ++++----
.../ExpressionParser/Clang/ClangUserExpression.cpp | 6 ++++--
.../Expr/TestClangModuleLoadError_InvalidSearchPath.test | 3 +--
.../TestClangModuleLoadError_InvalidTopLevelModule.test | 5 +++--
.../Expr/TestClangModuleLoadError_ModulemapParsing.test | 3 +--
.../Shell/Expr/TestClangModuleLoadError_NoModule.test | 3 +--
.../Shell/Expr/TestClangModuleLoadError_NoModuleMap.test | 3 +--
7 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index 6c5243b853ddf..7635e49051216 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -436,13 +436,13 @@ llvm::Error ClangModulesDeclVendorImpl::AddModulesForCompileUnit(
if (!LanguageSupportsClangModules(cu.GetLanguage()))
return llvm::Error::success();
+ llvm::Error errors = llvm::Error::success();
+
for (auto &imported_module : cu.GetImportedModules())
- // TODO: don't short-circuit. Continue loading modules even if one of them
- // fails. Concatenate all the errors.
if (auto err = AddModule(imported_module, &exported_modules))
- return err;
+ errors = llvm::joinErrors(std::move(errors), std::move(err));
- return llvm::Error::success();
+ return errors;
}
// ClangImporter::lookupValue
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 13d32b3bbc4f3..9a5d49a7ea363 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -381,8 +381,10 @@ static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target,
// FIXME: should we be dumping these to the error log instead of as
// diagnostics? They are non-fatal and are usually quite noisy.
- diagnostic_manager.PutString(lldb::eSeverityInfo,
- llvm::toString(std::move(err)));
+ llvm::handleAllErrors(
+ std::move(err), [&diagnostic_manager](const llvm::StringError &e) {
+ diagnostic_manager.PutString(lldb::eSeverityInfo, e.getMessage());
+ });
}
ClangExpressionSourceCode::WrapKind ClangUserExpression::GetWrapKind() const {
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
index 0fda05283608e..e5a6334d2ee22 100644
--- a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidSearchPath.test
@@ -43,5 +43,4 @@ run
expr blah
# CHECK: note: couldn't find module search path directory {{.*}}sources
-## FIXME: We never attempted to load bar.
-# CHECK-NOT: couldn't find module search path
+# CHECK: note: couldn't find module search path directory {{.*}}sources
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test
index a50f784dd1dc2..189a3c0fcdf26 100644
--- a/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_InvalidTopLevelModule.test
@@ -44,5 +44,6 @@ expr blah
# CHECK: note: couldn't load top-level module foo
## FIXME: clang error diagnostic shouldn't be dumped to the console.
# CHECK: error:
-## FIXME: We never attempted to load bar.
-# CHECK-NOT: bar
+# CHECK: note: couldn't load top-level module bar
+## FIXME: clang error diagnostic shouldn't be dumped to the console.
+# CHECK: error:
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test
index cadeb2de02c87..53efffffe435c 100644
--- a/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_ModulemapParsing.test
@@ -42,5 +42,4 @@ run
expr blah
# CHECK: note: failed to parse and load modulemap file in {{.*}}sources
-## FIXME: We never attempted to load bar.
-# CHECK-NOT: failed to parse and load modulemap
+# CHECK: note: failed to parse and load modulemap file in {{.*}}sources
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test
index a14f51eeba073..2962614086931 100644
--- a/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModule.test
@@ -43,5 +43,4 @@ run
expr blah
# CHECK: note: header search couldn't locate module 'foo'
-## FIXME: We never attempted to load bar.
-# CHECK-NOT: bar
+# CHECK: note: header search couldn't locate module 'bar'
diff --git a/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test
index a4fcace4f5b51..917facb05fd47 100644
--- a/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test
+++ b/lldb/test/Shell/Expr/TestClangModuleLoadError_NoModuleMap.test
@@ -38,5 +38,4 @@ run
expr blah
# CHECK: note: couldn't find modulemap file in {{.*}}sources
-## FIXME: We never attempted to load bar.
-# CHECK-NOT: couldn't find modulemap
+# CHECK: note: couldn't find modulemap file in {{.*}}sources
More information about the lldb-commits
mailing list