-
Notifications
You must be signed in to change notification settings - Fork 14.1k
[lldb][Modules] Make decls from submodules visible for name lookup #143098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as `Hidden`. When Clang asked LLDB for decls, it would see them in the submodule but `clang::Sema` would reject them because they weren't `Visible` (specifically, `getAcceptableDecl` would fail during `CppNameLookup`). Here we just mark the submodule as visible to work around this problem.
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as Full diff: https://github.com/llvm/llvm-project/pull/143098.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index c99ed9dd0a68d..284e5bc6fb1e5 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -382,6 +382,13 @@ bool ClangModulesDeclVendorImpl::AddModule(const SourceModule &module,
}
}
+ // If we didn't make the submodule visible here, Clang wouldn't allow LLDB to
+ // pick any of the decls in the submodules during C++ name lookup.
+ if (submodule)
+ m_compiler_instance->makeModuleVisible(
+ submodule, clang::Module::NameVisibilityKind::AllVisible,
+ /*ImportLoc=*/{});
+
clang::Module *requested_module = DoGetModule(clang_path, true);
if (requested_module != nullptr) {
diff --git a/lldb/test/API/lang/cpp/decl-from-submodule/TestDeclFromSubmodule.py b/lldb/test/API/lang/cpp/decl-from-submodule/TestDeclFromSubmodule.py
index f200e51c7f794..26ef12adf139a 100644
--- a/lldb/test/API/lang/cpp/decl-from-submodule/TestDeclFromSubmodule.py
+++ b/lldb/test/API/lang/cpp/decl-from-submodule/TestDeclFromSubmodule.py
@@ -15,13 +15,5 @@ def test_expr(self):
self.build()
lldbutil.run_to_source_breakpoint(self, "return 0", lldb.SBFileSpec("main.cpp"))
- # FIXME: LLDB finds the decl for 'func' in the submodules correctly and hands it to Clang
- # but Sema rejects using the decl during name lookup because it is not marked "Visible".
- # However, this assertions still ensures that we at least don't fail to compile the
- # submodule (which would cause other errors to appear before the expression error, hence
- # we use "startstr").
- self.expect(
- "expr func(1, 2)",
- error=True,
- startstr="error: <user expression 0>:1:1: 'func' has unknown return type",
- )
+ self.expect_expr("func(1, 2)", result_type=int, result_value=3)
+ self.expect_expr("func(1)", result_type=int, result_value=1)
|
@@ -11,17 +11,11 @@ | |||
class DeclFromSubmoduleTestCase(TestBase): | |||
# Requires DWARF debug info which is not retained when linking with link.exe. | |||
@skipIfWindows | |||
# Lookup for decls in submodules fails in Linux | |||
@expectedFailureAll(oslist=["linux"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really Linux or local submodule visibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably local submodule visibility. Just didn't have the time to investigate this on Linux
…lvm#143098) This patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as `Hidden`. When Clang asked LLDB for decls, it would see them in the submodule but `clang::Sema` would reject them because they weren't `Visible` (specifically, `getAcceptableDecl` would fail during `CppNameLookup`). Here we just mark the submodule as visible to work around this problem. (cherry picked from commit 30f5240)
…lvm#143098) This patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as `Hidden`. When Clang asked LLDB for decls, it would see them in the submodule but `clang::Sema` would reject them because they weren't `Visible` (specifically, `getAcceptableDecl` would fail during `CppNameLookup`). Here we just mark the submodule as visible to work around this problem.
…lvm#143098) This patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as `Hidden`. When Clang asked LLDB for decls, it would see them in the submodule but `clang::Sema` would reject them because they weren't `Visible` (specifically, `getAcceptableDecl` would fail during `CppNameLookup`). Here we just mark the submodule as visible to work around this problem.
…lvm#143098) This patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as `Hidden`. When Clang asked LLDB for decls, it would see them in the submodule but `clang::Sema` would reject them because they weren't `Visible` (specifically, `getAcceptableDecl` would fail during `CppNameLookup`). Here we just mark the submodule as visible to work around this problem.
This patch ensures we can find decls in submodules during expression evaluation. Previously, submodules would have all their decls marked as
Hidden
. When Clang asked LLDB for decls, it would see them in the submodule butclang::Sema
would reject them because they weren'tVisible
(specifically,getAcceptableDecl
would fail duringCppNameLookup
). Here we just mark the submodule as visible to work around this problem.