[lldb][modern-type-lookup] Ask the ExternalASTMerger to lookup namespaces instead of using the old mechanism

[teemperor]

Summary

With this patch LLDB starts querying the ExternalASTMerger when we look up a namespace in the translation unit context. Because
Clang's FindExternalVisibleDecls is returning a true/false on Lookup and LLDB instead uses a list of decls, this also adds some
glue code that passes this bool back to the caller when we have modern-type-lookup activated. We can't easily emulate
LLDB's API as Clang doesn't directly tell us what declarations it found in the DeclContext and querying the decl context for potential
declarations could have side-effects (like modifying the cached lookup in some way).

Note that this doesn't change any code for normal LLDB as we continue to ignore that return value in normal LLDB (mostly because
I don't know if there is a reason this was implemented this way).

The bigger change in ClangExpressionDeclMap.cpp is necessary as currently never query the ExternalASTSource when we don't
have a NamespaceMap (which we never have, as that's a non-modern-type-lookup thing). So instead we just ignore the NameSpaceMap
and continue to the actual ExternalASTSource in this case.

[teemperor]

modern-type-lookup has been removed, closing this.

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/Makefile

+CXX_SOURCES := main.cpp
+include Makefile.rules

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/TestNamespaceWithModernTypeLookup.py

+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class NamespaceWithModernTypeLookup(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test(self):
+        self.build()
+
+        # Activate modern-type-lookup.
+        self.runCmd("settings set target.experimental.use-modern-type-lookup true")
+
+        lldbutil.run_to_source_breakpoint(self,
+            "// Set break point at this line.", lldb.SBFileSpec("main.cpp"))
+
+        # Test lookup in namespace.
+        self.expect("expr A::i", substrs=["(int) ", " = 42\n"])

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/main.cpp

+namespace A { int i = 42; }
+
+int main(int argc, char **argv) {
+  return A::i; // Set break point at this line.
+}

lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h

public:
   clang::NamespaceDecl *
   AddNamespace(NameSearchContext &context,
                ClangASTImporter::NamespaceMapSP &namespace_decls);
 
   /// The worker function for FindExternalVisibleDeclsByName.
   ///
   /// \param[in] context
   ///     The NameSearchContext to use when filing results.
-  virtual void FindExternalVisibleDecls(NameSearchContext &context);
+  virtual bool FindExternalVisibleDecls(NameSearchContext &context);
 
   clang::Sema *getSema();
 
   void SetImportInProgress(bool import_in_progress) {
     m_import_in_progress = import_in_progress;
   }
   bool GetImportInProgress() { return m_import_in_progress; }
 

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

bool ClangASTSource::FindExternalVisibleDeclsByName(
   m_active_lookups.insert(uniqued_const_decl_name);
   //  static uint32_t g_depth = 0;
   //  ++g_depth;
   //  printf("[%5u] FindExternalVisibleDeclsByName() \"%s\"\n", g_depth,
   //  uniqued_const_decl_name);
   llvm::SmallVector<NamedDecl *, 4> name_decls;
   NameSearchContext name_search_context(*this, name_decls, clang_decl_name,
                                         decl_ctx);
-  FindExternalVisibleDecls(name_search_context);
+  bool success = FindExternalVisibleDecls(name_search_context);
   SetExternalVisibleDeclsForName(decl_ctx, clang_decl_name, name_decls);
   //  --g_depth;
   m_active_lookups.erase(uniqued_const_decl_name);
+  // The merger properly implements FindExternalVisibleDecls and returns
+  // true/false when it found something. There is still some non-modern-type
+  // lookup code in FindExternalVisibleDecls that could return a wrong value
+  // for success, so let's not trust the return value when we don't use
+  // modern-type-lookup.
+  if (HasMerger())
+    return success || (name_decls.size() != 0);
   return (name_decls.size() != 0);
 }
 
 void ClangASTSource::CompleteType(TagDecl *tag_decl) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   static unsigned int invocation_id = 0;
   unsigned int current_id = invocation_id++;
if (SkippedDecls) {
     // ensure that the lookup table is rebuilt, which means the external source
     // is consulted again when a clang::DeclContext::lookup is called.
     const_cast<DeclContext *>(decl_context)->setMustBuildLookupTable();
   }
 
   return;
 }
 
-void ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context) {
+bool ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context) {
   assert(m_ast_context);
 
   ClangASTMetrics::RegisterVisibleQuery();
 
   const ConstString name(context.m_decl_name.getAsString().c_str());
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
if (log) {
     else
       LLDB_LOGF(log,
                 "ClangASTSource::FindExternalVisibleDecls[%u] on "
                 "(ASTContext*)%p for '%s' in a '%s'",
                 current_id, static_cast<void *>(m_ast_context),
                 name.GetCString(), context.m_decl_context->getDeclKindName());
   }
 
-  if (HasMerger() && !isa<TranslationUnitDecl>(context.m_decl_context)
-      /* possibly handle NamespaceDecls here? */) {
+  if (HasMerger()) {
     if (auto *interface_decl =
     dyn_cast<ObjCInterfaceDecl>(context.m_decl_context)) {
       ObjCInterfaceDecl *complete_iface_decl =
       GetCompleteObjCInterface(interface_decl);
 
       if (complete_iface_decl && (complete_iface_decl != interface_decl)) {
         GetMergerUnchecked().ForceRecordOrigin(
             interface_decl,
             {complete_iface_decl, &complete_iface_decl->getASTContext()});
       }
     }
 
-    GetMergerUnchecked().FindExternalVisibleDeclsByName(context.m_decl_context,
-                                                context.m_decl_name);
-    return; // otherwise we may need to fall back
+    if (GetMergerUnchecked().FindExternalVisibleDeclsByName(
+            context.m_decl_context, context.m_decl_name))
+      return true;
   }
 
   context.m_namespace_map = std::make_shared<ClangASTImporter::NamespaceMap>();
 
   if (const NamespaceDecl *namespace_context =
           dyn_cast<NamespaceDecl>(context.m_decl_context)) {
     ClangASTImporter::NamespaceMapSP namespace_map =  m_ast_importer_sp ?
         m_ast_importer_sp->GetNamespaceMap(namespace_context) : nullptr;
 
     if (log && log->GetVerbose())
       LLDB_LOGF(log, "  CAS::FEVD[%u] Inspecting namespace map %p (%d entries)",
                 current_id, static_cast<void *>(namespace_map.get()),
                 static_cast<int>(namespace_map->size()));
 
     if (!namespace_map)
-      return;
+      return false;
 
     for (ClangASTImporter::NamespaceMap::iterator i = namespace_map->begin(),
                                                   e = namespace_map->end();
          i != e; ++i) {
       LLDB_LOGF(log, "  CAS::FEVD[%u] Searching namespace %s in module %s",
                 current_id, i->second.GetName().AsCString(),
                 i->first->GetFileSpec().GetFilename().GetCString());
 
       FindExternalVisibleDecls(context, i->first, i->second, current_id);
     }
   } else if (isa<ObjCInterfaceDecl>(context.m_decl_context) && !HasMerger()) {
     FindObjCPropertyAndIvarDecls(context);
   } else if (!isa<TranslationUnitDecl>(context.m_decl_context)) {
     // we shouldn't be getting FindExternalVisibleDecls calls for these
-    return;
+    return false;
   } else {
     CompilerDeclContext namespace_decl;
 
     LLDB_LOGF(log, "  CAS::FEVD[%u] Searching the root namespace", current_id);
 
     FindExternalVisibleDecls(context, lldb::ModuleSP(), namespace_decl,
                              current_id);
   }
 
   if (!context.m_namespace_map->empty()) {
     if (log && log->GetVerbose())
       LLDB_LOGF(log,
                 "  CAS::FEVD[%u] Registering namespace map %p (%d entries)",
                 current_id, static_cast<void *>(context.m_namespace_map.get()),
                 static_cast<int>(context.m_namespace_map->size()));
 
     NamespaceDecl *clang_namespace_decl =
         AddNamespace(context, context.m_namespace_map);
 
     if (clang_namespace_decl)
       clang_namespace_decl->setHasExternalVisibleStorage();
   }
+  return false;
 }
 
 clang::Sema *ClangASTSource::getSema() {
   return ClangASTContext::GetASTContext(m_ast_context)->getSema();
 }
 
 bool ClangASTSource::IgnoreName(const ConstString name,
                                 bool ignore_all_dollar_names) {

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h

public:
   /// [Used by ClangASTSource] Find all entities matching a given name, using
   /// a NameSearchContext to make Decls for them.
   ///
   /// \param[in] context
   ///     The NameSearchContext that can construct Decls for this name.
   ///
   /// \return
   ///     True on success; false otherwise.
-  void FindExternalVisibleDecls(NameSearchContext &context) override;
+  bool FindExternalVisibleDecls(NameSearchContext &context) override;
 
   /// Find all entities matching a given name in a given module/namespace,
   /// using a NameSearchContext to make Decls for them.
   ///
   /// \param[in] context
   ///     The NameSearchContext that can construct Decls for this name.
   ///
   /// \param[in] module

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

if (!frame_decl_context)
     return nullptr;
 
   return llvm::dyn_cast_or_null<ClangASTContext>(
       frame_decl_context.GetTypeSystem());
 }
 
 // Interface for ClangASTSource
 
-void ClangExpressionDeclMap::FindExternalVisibleDecls(
+bool ClangExpressionDeclMap::FindExternalVisibleDecls(
     NameSearchContext &context) {
   assert(m_ast_context);
 
   ClangASTMetrics::RegisterVisibleQuery();
 
   const ConstString name(context.m_decl_name.getAsString().c_str());
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   if (GetImportInProgress()) {
     if (log && log->GetVerbose())
       LLDB_LOGF(log, "Ignoring a query during an import");
-    return;
+    return false;
   }
 
   static unsigned int invocation_id = 0;
   unsigned int current_id = invocation_id++;
 
   if (log) {
     if (!context.m_decl_context)
       LLDB_LOGF(log,
if (const NamespaceDecl *namespace_context =
           dyn_cast<NamespaceDecl>(context.m_decl_context)) {
     if (namespace_context->getName().str() ==
         std::string(g_lldb_local_vars_namespace_cstr)) {
       CompilerDeclContext compiler_decl_ctx(
           GetClangASTContext(), const_cast<void *>(static_cast<const void *>(
                                     context.m_decl_context)));
       FindExternalVisibleDecls(context, lldb::ModuleSP(), compiler_decl_ctx,
                                current_id);
-      return;
+      return context.m_decls.size() != 0;
     }
 
+    // Only do this when we have a ClangASTImporter.
+    if (m_ast_importer_sp) {
       ClangASTImporter::NamespaceMapSP namespace_map =
-        m_ast_importer_sp
-            ? m_ast_importer_sp->GetNamespaceMap(namespace_context)
-            : ClangASTImporter::NamespaceMapSP();
+          m_ast_importer_sp->GetNamespaceMap(namespace_context);
 
       if (!namespace_map)
-      return;
+        return context.m_decls.size() != 0;
 
       if (log && log->GetVerbose())
         log->Printf("  CEDM::FEVD[%u] Inspecting (NamespaceMap*)%p (%d entries)",
                     current_id, static_cast<void *>(namespace_map.get()),
                     (int)namespace_map->size());
 
       for (ClangASTImporter::NamespaceMap::iterator i = namespace_map->begin(),
                                                     e = namespace_map->end();
            i != e; ++i) {
         if (log)
           log->Printf("  CEDM::FEVD[%u] Searching namespace %s in module %s",
                       current_id, i->second.GetName().AsCString(),
                       i->first->GetFileSpec().GetFilename().GetCString());
 
         FindExternalVisibleDecls(context, i->first, i->second, current_id);
       }
+    }
+
   } else if (isa<TranslationUnitDecl>(context.m_decl_context)) {
     CompilerDeclContext namespace_decl;
 
     if (log)
       log->Printf("  CEDM::FEVD[%u] Searching the root namespace", current_id);
 
     FindExternalVisibleDecls(context, lldb::ModuleSP(), namespace_decl,
                              current_id);
   }
 
-  ClangASTSource::FindExternalVisibleDecls(context);
+  return ClangASTSource::FindExternalVisibleDecls(context);
 }
 
 void ClangExpressionDeclMap::FindExternalVisibleDecls(
     NameSearchContext &context, lldb::ModuleSP module_sp,
     CompilerDeclContext &namespace_decl, unsigned int current_id) {
   assert(m_ast_context);
 
   std::function<void(clang::FunctionDecl *)> MaybeRegisterFunctionBody =