This revision was automatically updated to reflect the committed changes.
Closed by commit rG6fcb857746c1: [lldb][import-std-module] Prefer the
non-module diagnostics when in fallback… (authored by teemperor).
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110696/new/
https://reviews.llvm.org/D110696
Files:
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/test/API/commands/expression/import-std-module/module-build-errors/Makefile
lldb/test/API/commands/expression/import-std-module/module-build-errors/TestStdModuleBuildErrors.py
lldb/test/API/commands/expression/import-std-module/module-build-errors/main.cpp
lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/algorithm
lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/module.modulemap
lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/vector
lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/stdio.h
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
Index: lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
===================================================================
--- lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
@@ -48,16 +48,6 @@
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'"])
Index: lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/stdio.h
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/stdio.h
@@ -0,0 +1 @@
+struct libc_struct {};
Index: lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/module.modulemap
===================================================================
--- /dev/null
+++ 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 * }
+}
Index: lldb/test/API/commands/expression/import-std-module/module-build-errors/root/usr/include/c++/v1/algorithm
===================================================================
--- /dev/null
+++ 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<typename T>
+ 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
Index: lldb/test/API/commands/expression/import-std-module/module-build-errors/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/module-build-errors/main.cpp
@@ -0,0 +1,8 @@
+#include <algorithm>
+
+int main(int argc, char **argv) {
+ // Makes sure we have the mock libc headers in the debug information.
+ libc_struct s;
+ std::vector<int> v;
+ return 0; // Set break point at this line.
+}
Index: lldb/test/API/commands/expression/import-std-module/module-build-errors/TestStdModuleBuildErrors.py
===================================================================
--- /dev/null
+++ 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)
Index: lldb/test/API/commands/expression/import-std-module/module-build-errors/Makefile
===================================================================
--- /dev/null
+++ 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
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -685,15 +685,22 @@
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)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
