[lldb][Modules] Make decls from submodules visible for name lookup

[Michael137]

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.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/143098.diff

2 Files Affected:

• (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (+7)
• (modified) lldb/test/API/lang/cpp/decl-from-submodule/TestDeclFromSubmodule.py (+2-10)

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)

[adrian-prantl]

Is it really Linux or local submodule visibility?

[Michael137]

Probably local submodule visibility. Just didn't have the time to investigate this on Linux