diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index 41e86a1f897e9..38166391006a0 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -685,15 +685,22 @@ bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, SetupCppModuleImports(exe_ctx); // If we did load any modules, then retry parsing. if (!m_imported_cpp_modules.empty()) { + // Create a dedicated diagnostic manager for the second parse attempt. + // These diagnostics are only returned to the caller if using the fallback + // actually succeeded in getting the expression to parse. This prevents + // that module-specific issues regress diagnostic quality with the + // fallback mode. + DiagnosticManager retry_manager; // The module imports are injected into the source code wrapper, // so recreate those. - CreateSourceCode(diagnostic_manager, exe_ctx, m_imported_cpp_modules, + CreateSourceCode(retry_manager, exe_ctx, m_imported_cpp_modules, /*for_completion*/ false); - // Clear the error diagnostics from the previous parse attempt. - diagnostic_manager.Clear(); - parse_success = TryParse(diagnostic_manager, exe_scope, exe_ctx, + parse_success = TryParse(retry_manager, exe_scope, exe_ctx, execution_policy, keep_result_in_memory, generate_debug_info); + // Return the parse diagnostics if we were successful. + if (parse_success) + diagnostic_manager = std::move(retry_manager); } } if (!parse_success) diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/Makefile b/lldb/test/API/commands/expression/import-std-module/module-build-errors/Makefile new file mode 100644 index 0000000000000..617045d760cc4 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/module-build-errors/Makefile @@ -0,0 +1,9 @@ +# We don't have any standard include directories, so we can't +# parse the test_common.h header we usually inject as it includes +# system headers. +NO_TEST_COMMON_H := 1 + +CXXFLAGS_EXTRAS = -I $(SRCDIR)/root/usr/include/c++/v1/ -I $(SRCDIR)/root/usr/include/ -nostdinc -nostdinc++ -DBUILDING_OUTSIDE_LLDB=1 +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/TestStdModuleBuildErrors.py b/lldb/test/API/commands/expression/import-std-module/module-build-errors/TestStdModuleBuildErrors.py new file mode 100644 index 0000000000000..55e6d420b64b7 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/module-build-errors/TestStdModuleBuildErrors.py @@ -0,0 +1,61 @@ +""" +Tests that the import-std-module=fallback setting is only showing the error +diagnostics from the first parse attempt which isn't using a module. +This is supposed to prevent that a broken libc++ module renders failing +expressions useless as the original failing errors are suppressed by the +module build errors. +""" + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +import os + + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + # We only emulate a fake libc++ in this test and don't use the real libc++, + # but we still add the libc++ category so that this test is only run in + # test configurations where libc++ is actually supposed to be tested. + @add_test_categories(["libc++"]) + @skipIfRemote + @skipIf(compiler=no_match("clang")) + def test(self): + self.build() + + sysroot = os.path.join(os.getcwd(), "root") + + # Set the sysroot this test is using to provide a custom libc++. + self.runCmd("platform select --sysroot '" + sysroot + "' host", + CURRENT_EXECUTABLE_SET) + + lldbutil.run_to_source_breakpoint(self, + "// Set break point at this line.", + lldb.SBFileSpec("main.cpp")) + + # The expected error message when the fake libc++ module in this test + # fails to build from within LLDB (as it contains invalid code). + module_build_error_msg = "unknown type name 'random_token_to_fail_the_build'" + + # First force the std module to be imported. This should show the + # module build error to the user. + self.runCmd("settings set target.import-std-module true") + self.expect("expr (size_t)v.size()", + substrs=[module_build_error_msg], + error=True) + + # In the fallback mode the module build error should not be shown. + self.runCmd("settings set target.import-std-module fallback") + fallback_expr = "expr v ; error_to_trigger_fallback_mode" + # First check for the actual expression error that should be displayed + # and is useful for the user. + self.expect(fallback_expr, + substrs=["use of undeclared identifier 'error_to_trigger_fallback_mode'"], + error=True) + # Test that the module build error is not displayed. + self.expect(fallback_expr, + substrs=[module_build_error_msg], + matching=False, + error=True) diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/main.cpp b/lldb/test/API/commands/expression/import-std-module/module-build-errors/main.cpp new file mode 100644 index 0000000000000..b01fe1a785361 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/module-build-errors/main.cpp @@ -0,0 +1,8 @@ +#include + +int main(int argc, char **argv) { + // Makes sure we have the mock libc headers in the debug information. + libc_struct s; + std::vector v; + return 0; // Set break point at this line. +} diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/algorithm b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/algorithm new file mode 100644 index 0000000000000..af8738f075b30 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/algorithm @@ -0,0 +1,18 @@ +// This is only defined when building, but LLDB is missing this flag when loading the standard +// library module so the actual contents of the module are missing. +#ifdef BUILDING_OUTSIDE_LLDB + +#include "stdio.h" + +namespace std { + inline namespace __1 { + // Pretend to be a std::vector template we need to instantiate + // in LLDB. + template + struct vector { T i; int size() { return 2; } }; + } +} +#else +// Break the build when parsing from within LLDB. +random_token_to_fail_the_build +#endif // BUILDING_OUTSIDE_LLDB diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/module.modulemap b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/module.modulemap new file mode 100644 index 0000000000000..0eb48492a65d9 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/module.modulemap @@ -0,0 +1,3 @@ +module std { + module "algorithm" { header "algorithm" export * } +} diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/vector b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/vector new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/stdio.h b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/stdio.h new file mode 100644 index 0000000000000..47525c9db3467 --- /dev/null +++ b/lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/stdio.h @@ -0,0 +1 @@ +struct libc_struct {}; diff --git a/lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py b/lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py index b84ab84e4b480..1ba80ca1d011b 100644 --- a/lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py +++ b/lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py @@ -48,16 +48,6 @@ def test(self): self.expect("expr --top-level -- int i = std::max(1, 2);", error=True, substrs=["no member named 'max' in namespace 'std'"]) - # Check that diagnostics from the first parse attempt don't show up - # in the C++ module parse attempt. In the expression below, we first - # fail to parse 'std::max'. Then we retry with a loaded C++ module - # and succeed to parse the 'std::max' part. However, the - # trailing 'unknown_identifier' will fail to parse even with the - # loaded module. The 'std::max' diagnostic from the first attempt - # however should not be shown to the user. - self.expect("expr std::max(1, 2); unknown_identifier", error=True, - matching=False, - substrs=["no member named 'max'"]) # The proper diagnostic however should be shown on the retry. self.expect("expr std::max(1, 2); unknown_identifier", error=True, substrs=["use of undeclared identifier 'unknown_identifier'"])