[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:
* llvm#166917

Author: Michael137

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

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 {

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

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:

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

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'

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