[lldb][ClangModulesDeclVendor] Print Clang module loading errors to expression log instead of console (#166964)

Depends on:
* #166917
* #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.

Author: Michael137

lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

@@ -379,12 +379,12 @@ static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target,
   if (!err)
     return;
 
-  // FIXME: should we be dumping these to the error log instead of as
-  // diagnostics? They are non-fatal and are usually quite noisy.
-  llvm::handleAllErrors(
-      std::move(err), [&diagnostic_manager](const llvm::StringError &e) {
-        diagnostic_manager.PutString(lldb::eSeverityInfo, e.getMessage());
-      });
+  // Module load errors aren't fatal to the expression evaluator. Printing
+  // them as diagnostics to the console would be too noisy and misleading
+  // Hence just print them to the expression log.
+  llvm::handleAllErrors(std::move(err), [](const llvm::StringError &e) {
+    LLDB_LOG(GetLog(LLDBLog::Expressions), "{0}", e.getMessage());
+  });
 }
 
 ClangExpressionSourceCode::WrapKind ClangUserExpression::GetWrapKind() const {

lldb/test/API/lang/objc/modules-compile-error/Makefile

@@ -0,0 +1,46 @@
+## Tests the case where module compilation fails.
+#
+# REQUIRES: system-darwin
+#
+# RUN: split-file %s %t/sources
+# RUN: %clang_host -g %t/sources/main.m -fmodules -fcxx-modules \
+# RUN:             -DSHOULD_COMPILE=1 \
+# RUN:             -fmodule-map-file=%t/sources/module.modulemap \
+# RUN:             -fmodules-cache-path=%t/ModuleCache -o %t.out
+#
+# 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;
+
+int main() { __builtin_debugtrap(); }
+
+#--- foo.h
+struct foo {};
+
+#ifndef SHOULD_COMPILE
+#error "Compilation failure."
+#endif
+
+#--- module.modulemap
+module foo {
+    header "foo.h"
+    export *
+}
+
+#--- commands.input
+log enable lldb expr
+run
+## Make sure expression fails so the 'note' diagnostics get printed.
+expr blah
+
+# CHECK: Finished building Clang module foo
+# CHECK: couldn't load top-level module foo:
+# CHECK: While building module 'foo' imported from LLDBModulesMemoryBuffer
+# CHEKC: {{.*}}sources/foo.h{{.*}}: error: "Compilation failure."
+# CHECK: LLDBModulesMemoryBuffer:1:1: fatal error: could not build module 'foo'
+
+# CHECK: Error while loading hand-imported modules:
+# CHECK: couldn't load top-level module foo:
+# CHECK-NOT: Compilation failure

lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py

@@ -10,7 +10,10 @@
 # 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
+# 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;
@@ -56,4 +59,12 @@ run
 ## Make sure expression fails so the 'note' diagnostics get printed.
 expr blah
 
-# CHECK: note: couldn't load submodule 'qux' of module 'foo.baz'
+# 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'

lldb/test/API/lang/objc/modules-compile-error/main.m

@@ -9,10 +9,14 @@
 # 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
+# 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;
@@ -42,5 +46,14 @@ run
 ## Make sure expression fails so the 'note' diagnostics get printed.
 expr blah
 
-# CHECK: note: couldn't find module search path directory {{.*}}sources
-# CHECK: note: couldn't find module search path directory {{.*}}sources
+# 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

lldb/test/API/lang/objc/modules-compile-error/module.h

@@ -10,7 +10,10 @@
 # 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
+# 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;
@@ -48,4 +51,12 @@ run
 ## Make sure expression fails so the 'note' diagnostics get printed.
 expr blah
 
-# CHECK: note: couldn't load submodule 'baz' of module 'foo'
+# NO_LOG-NOT: couldn't load submodule 'baz' of module 'foo'
+
+#--- 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 'baz' of module 'foo'