https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/166940
>From 89c1cd5cb28235f2a24fbd5165ed58ad82229666 Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Fri, 7 Nov 2025 13:53:51 +0000 Subject: [PATCH 1/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 >From 39856eab4abfa58c8351e4cf2c91d1092786783b Mon Sep 17 00:00:00 2001 From: Michael Buch <[email protected]> Date: Fri, 7 Nov 2025 14:31:04 +0000 Subject: [PATCH 2/2] fixup! add newline now that we're printing more than a single error --- .../ExpressionParser/Clang/ClangExpressionSourceCode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp index b4e81aa21c138..ad48d293ab8f0 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp @@ -387,7 +387,7 @@ bool ClangExpressionSourceCode::GetText( *sc.comp_unit, modules_for_macros)) LLDB_LOG_ERROR( GetLog(LLDBLog::Expressions), std::move(err), - "Error while loading hand-imported modules: {0}"); + "Error while loading hand-imported modules:\n{0}"); } } } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
