[lldb][ClangModulesDeclVendor] Don't stop loading Clang modules if an individual import failed (#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 case 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.
This commit is contained in:
Michael Buch 2025-11-07 23:11:48 +00:00 committed by GitHub
parent bbc4a450c6
commit 74e34eff3f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 18 additions and 19 deletions

View File

@ -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}");
}
}
}

View File

@ -436,13 +436,13 @@ llvm::Error ClangModulesDeclVendorImpl::AddModulesForCompileUnit(
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;
llvm::Error errors = llvm::Error::success();
return 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

View File

@ -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 {

View File

@ -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

View File

@ -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:

View File

@ -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

View File

@ -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'

View File

@ -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