[lldb] Move ClangModulesDeclVendor ownership to ClangPersistentVariables from Target

[bulbazord]

Summary

More decoupling of plugins and non-plugins. Target doesn't need to
manage ClangModulesDeclVendor and ClangPersistentVariables is always available
in situations where you need ClangModulesDeclVendor.

[teemperor]

First and foremost: Cool new username. +1 from me.

Otherwise I think Shafik's comment in D79752 still applies here. Could you make a short utility function for that chunk of code and put in ClangASTSource (ClangExpressionDeclMap inherits from this, so you can remove that part form both classes):

auto persistent_vars = llvm::cast<ClangPersistentVariables>(
    m_target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
std::shared_ptr<ClangModulesDeclVendor> modules_decl_vendor =
    persistent_vars->GetClangModulesDeclVendor();

Just something like ClangModulesDeclVendor &ClangASTSource::GetClangModulesDeclVendor() { ... } seems fine.

Beside that I think this is good to go.

[JDevlieghere]

In D102811#2769686, @teemperor wrote:

First and foremost: Cool new username. +1 from me.

+2

[bulbazord]

Add helper in ClangASTSource

[teemperor]

LGTM. Soon LLDB will be as nicely layered (and maybe as tasty) as some good Schichttorte

lldb/include/lldb/Target/Target.h

 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Timeout.h"
 #include "lldb/lldb-public.h"
 
 namespace lldb_private {
 
-class ClangModulesDeclVendor;
-
 OptionEnumValues GetDynamicValueTypes();
 
 enum InlineStrategy {
   eInlineBreakpointsNever = 0,
   eInlineBreakpointsHeaders,
   eInlineBreakpointsAlways
 };
 
public:
   lldb::PlatformSP GetPlatform() { return m_platform_sp; }
 
   void SetPlatform(const lldb::PlatformSP &platform_sp) {
     m_platform_sp = platform_sp;
   }
 
   SourceManager &GetSourceManager();
 
-  ClangModulesDeclVendor *GetClangModulesDeclVendor();
-
   // Methods.
   lldb::SearchFilterSP
   GetSearchFilterForModule(const FileSpec *containingModule);
 
   lldb::SearchFilterSP
   GetSearchFilterForModuleList(const FileSpecList *containingModuleList);
 
   lldb::SearchFilterSP
protected:
   lldb::ProcessSP m_process_sp;
   lldb::SearchFilterSP m_search_filter_sp;
   PathMappingList m_image_search_paths;
   TypeSystemMap m_scratch_type_system_map;
 
   typedef std::map<lldb::LanguageType, lldb::REPLSP> REPLMap;
   REPLMap m_repl_map;
 
-  std::unique_ptr<ClangModulesDeclVendor> m_clang_modules_decl_vendor_up;
-
   lldb::SourceManagerUP m_source_manager_up;
 
   typedef std::map<lldb::user_id_t, StopHookSP> StopHookCollection;
   StopHookCollection m_stop_hooks;
   lldb::user_id_t m_stop_hook_next_id;
   bool m_valid;
   bool m_suppress_stop_hooks;
   bool m_is_dummy_target;

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

protected:
   ///
   /// \param[in] src_type
   ///     The source type.
   ///
   /// \return
   ///     The imported type.
   CompilerType GuardedCopyType(const CompilerType &src_type);
 
+  std::shared_ptr<ClangModulesDeclVendor> GetClangModulesDeclVendor();
+
 public:
   /// Returns true if a name should be ignored by name lookup.
   ///
   /// \param[in] name
   ///     The name to be considered.
   ///
   /// \param[in] ignore_all_dollar_names
   ///     True if $-names of all sorts should be ignored.

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

bool ClangASTSource::FindObjCMethodDeclsWithOrigin(
 
   return true;
 }
 
 void ClangASTSource::FindDeclInModules(NameSearchContext &context,
                                        ConstString name) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
-  ClangModulesDeclVendor *modules_decl_vendor =
-      m_target->GetClangModulesDeclVendor();
+  std::shared_ptr<ClangModulesDeclVendor> modules_decl_vendor =
+      GetClangModulesDeclVendor();
   if (!modules_decl_vendor)
     return;
 
   bool append = false;
   uint32_t max_matches = 1;
   std::vector<clang::NamedDecl *> decls;
 
   if (!modules_decl_vendor->FindDecls(name, append, max_matches, decls))
do {
 
     return;
   } while (false);
 
   do {
     // Check the modules only if the debug information didn't have a complete
     // interface.
 
-    if (ClangModulesDeclVendor *modules_decl_vendor =
-            m_target->GetClangModulesDeclVendor()) {
+    if (std::shared_ptr<ClangModulesDeclVendor> modules_decl_vendor =
+            GetClangModulesDeclVendor()) {
       ConstString interface_name(interface_decl->getNameAsString().c_str());
       bool append = false;
       uint32_t max_matches = 1;
       std::vector<clang::NamedDecl *> decls;
 
       if (!modules_decl_vendor->FindDecls(interface_name, append, max_matches,
                                           decls))
         break;
do {
 
     return;
   } while (false);
 
   do {
     // Check the modules only if the debug information didn't have a complete
     // interface.
 
-    ClangModulesDeclVendor *modules_decl_vendor =
-        m_target->GetClangModulesDeclVendor();
+    std::shared_ptr<ClangModulesDeclVendor> modules_decl_vendor =
+        GetClangModulesDeclVendor();
 
     if (!modules_decl_vendor)
       break;
 
     bool append = false;
     uint32_t max_matches = 1;
     std::vector<clang::NamedDecl *> decls;
 
CompilerType ClangASTSource::GuardedCopyType(const CompilerType &src_type) {
   if (copied_qual_type.getAsOpaquePtr() &&
       copied_qual_type->getCanonicalTypeInternal().isNull())
     // this shouldn't happen, but we're hardening because the AST importer
     // seems to be generating bad types on occasion.
     return CompilerType();
 
   return m_clang_ast_context->GetType(copied_qual_type);
 }
+
+std::shared_ptr<ClangModulesDeclVendor>
+ClangASTSource::GetClangModulesDeclVendor() {
+  auto persistent_vars = llvm::cast<ClangPersistentVariables>(
+      m_target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
+  return persistent_vars->GetClangModulesDeclVendor();
+}

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

 
 void ClangExpressionDeclMap::LookupInModulesDeclVendor(
     NameSearchContext &context, ConstString name) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   if (!m_target)
     return;
 
-  auto *modules_decl_vendor = m_target->GetClangModulesDeclVendor();
+  std::shared_ptr<ClangModulesDeclVendor> modules_decl_vendor =
+      GetClangModulesDeclVendor();
   if (!modules_decl_vendor)
     return;
 
   bool append = false;
   uint32_t max_matches = 1;
   std::vector<clang::NamedDecl *> decls;
 
   if (!modules_decl_vendor->FindDecls(name, append, max_matches, decls))
void ClangExpressionDeclMap::LookupFunction(
   if (!m_parser_vars)
     return;
 
   Target *target = m_parser_vars->m_exe_ctx.GetTargetPtr();
 
   std::vector<clang::NamedDecl *> decls_from_modules;
 
   if (target) {
-    if (ClangModulesDeclVendor *decl_vendor =
-            target->GetClangModulesDeclVendor()) {
+    if (std::shared_ptr<ClangModulesDeclVendor> decl_vendor =
+            GetClangModulesDeclVendor()) {
       decl_vendor->FindDecls(name, false, UINT32_MAX, decls_from_modules);
     }
   }
 
   const bool include_inlines = false;
   SymbolContextList sc_list;
   if (namespace_decl && module_sp) {
     const bool include_symbols = false;

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

case lldb::eLanguageTypeObjC:
     // Remove all C++ keywords from the PP so that the user can still use
     // variables that have C++ keywords as names (e.g. 'int template;').
     RemoveAllCppKeywords(m_compiler->getPreprocessor().getIdentifierTable());
     break;
   default:
     break;
   }
 
-  if (ClangModulesDeclVendor *decl_vendor =
-          target_sp->GetClangModulesDeclVendor()) {
-    if (auto *clang_persistent_vars = llvm::cast<ClangPersistentVariables>(
-            target_sp->GetPersistentExpressionStateForLanguage(
-                lldb::eLanguageTypeC))) {
+    if (std::shared_ptr<ClangModulesDeclVendor> decl_vendor =
+            clang_persistent_vars->GetClangModulesDeclVendor()) {
       std::unique_ptr<PPCallbacks> pp_callbacks(
           new LLDBPreprocessorCallbacks(*decl_vendor, *clang_persistent_vars,
                                         m_compiler->getSourceManager()));
       m_pp_callbacks =
           static_cast<LLDBPreprocessorCallbacks *>(pp_callbacks.get());
       m_compiler->getPreprocessor().addPPCallbacks(std::move(pp_callbacks));
     }
   }

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

if (target->GetArchitecture().GetMachine() == llvm::Triple::x86_64) {
       if (lldb::PlatformSP platform_sp = target->GetPlatform()) {
         static ConstString g_platform_ios_simulator("ios-simulator");
         if (platform_sp->GetPluginName() == g_platform_ios_simulator) {
           target_specific_defines = "typedef bool BOOL;\n";
         }
       }
     }
 
-    ClangModulesDeclVendor *decl_vendor = target->GetClangModulesDeclVendor();
     auto *persistent_vars = llvm::cast<ClangPersistentVariables>(
         target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
-    if (decl_vendor && persistent_vars) {
+    std::shared_ptr<ClangModulesDeclVendor> decl_vendor =
+        persistent_vars->GetClangModulesDeclVendor();
+    if (decl_vendor) {
       const ClangModulesDeclVendor::ModuleVector &hand_imported_modules =
           persistent_vars->GetHandLoadedClangModules();
       ClangModulesDeclVendor::ModuleVector modules_for_macros;
 
       for (ClangModulesDeclVendor::ModuleID module : hand_imported_modules) {
         modules_for_macros.push_back(module);
       }
 

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

 #include "ClangExpressionVariable.h"
 #include "ClangModulesDeclVendor.h"
 
 #include "lldb/Expression/ExpressionVariable.h"
 
 namespace lldb_private {
 
 class ClangASTImporter;
+class ClangModulesDeclVendor;
+class Target;
 class TypeSystemClang;
 
 /// \class ClangPersistentVariables ClangPersistentVariables.h
 /// "lldb/Expression/ClangPersistentVariables.h" Manages persistent values
 /// that need to be preserved between expression invocations.
 ///
 /// A list of variables that can be accessed and updated by any expression.  See
 /// ClangPersistentVariable for more discussion.  Also provides an increasing,
 /// 0-based counter for naming result variables.
 class ClangPersistentVariables : public PersistentExpressionState {
 public:
-  ClangPersistentVariables();
+  ClangPersistentVariables(std::shared_ptr<Target> target_sp);
 
   ~ClangPersistentVariables() override = default;
 
   // llvm casting support
   static bool classof(const PersistentExpressionState *pv) {
     return pv->getKind() == PersistentExpressionState::eKindClang;
   }
 
   std::shared_ptr<ClangASTImporter> GetClangASTImporter();
+  std::shared_ptr<ClangModulesDeclVendor> GetClangModulesDeclVendor();
 
   lldb::ExpressionVariableSP
   CreatePersistentVariable(const lldb::ValueObjectSP &valobj_sp) override;
 
   lldb::ExpressionVariableSP CreatePersistentVariable(
       ExecutionContextScope *exe_scope, ConstString name,
       const CompilerType &compiler_type, lldb::ByteOrder byte_order,
       uint32_t addr_byte_size) override;
private:
   PersistentDeclMap
       m_persistent_decls; ///< Persistent entities declared by the user.
 
   ClangModulesDeclVendor::ModuleVector
       m_hand_loaded_clang_modules; ///< These are Clang modules we hand-loaded;
                                    ///these are the highest-
                                    ///< priority source for macros.
   std::shared_ptr<ClangASTImporter> m_ast_importer_sp;
+  std::shared_ptr<ClangModulesDeclVendor> m_modules_decl_vendor_sp;
+  std::shared_ptr<Target> m_target_sp;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_EXPRESSIONPARSER_CLANG_CLANGPERSISTENTVARIABLES_H

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

 //===-- ClangPersistentVariables.cpp --------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ClangPersistentVariables.h"
 #include "ClangASTImporter.h"
+#include "ClangModulesDeclVendor.h"
 
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "clang/AST/Decl.h"
 
 #include "llvm/ADT/StringMap.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
-ClangPersistentVariables::ClangPersistentVariables()
-    : lldb_private::PersistentExpressionState(LLVMCastKind::eKindClang) {}
+ClangPersistentVariables::ClangPersistentVariables(
+    std::shared_ptr<Target> target_sp)
+    : lldb_private::PersistentExpressionState(LLVMCastKind::eKindClang),
+      m_target_sp(target_sp) {}
 
 ExpressionVariableSP ClangPersistentVariables::CreatePersistentVariable(
     const lldb::ValueObjectSP &valobj_sp) {
   return AddNewlyConstructedVariable(new ClangExpressionVariable(valobj_sp));
 }
 
 ExpressionVariableSP ClangPersistentVariables::CreatePersistentVariable(
     ExecutionContextScope *exe_scope, ConstString name,

 std::shared_ptr<ClangASTImporter>
 ClangPersistentVariables::GetClangASTImporter() {
   if (!m_ast_importer_sp) {
     m_ast_importer_sp = std::make_shared<ClangASTImporter>();
   }
   return m_ast_importer_sp;
 }
 
+std::shared_ptr<ClangModulesDeclVendor>
+ClangPersistentVariables::GetClangModulesDeclVendor() {
+  if (!m_modules_decl_vendor_sp) {
+    m_modules_decl_vendor_sp.reset(
+        ClangModulesDeclVendor::Create(*m_target_sp.get()));
+  }
+  return m_modules_decl_vendor_sp;
+}
+
 ConstString
 ClangPersistentVariables::GetNextPersistentVariableName(bool is_error) {
   llvm::SmallString<64> name;
   {
     llvm::raw_svector_ostream os(name);
     os << GetPersistentVariablePrefix(is_error)
        << m_next_persistent_variable_id++;
   }
   return ConstString(name);
 }

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

diagnostic_manager.PutString(eDiagnosticSeverityError,
                                  "error: couldn't start parsing (no target)");
     return false;
   }
   return true;
 }
 
 static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target,
                             DiagnosticManager &diagnostic_manager) {
-  ClangModulesDeclVendor *decl_vendor = target->GetClangModulesDeclVendor();
-  if (!decl_vendor)
-    return;
-
   if (!target->GetEnableAutoImportClangModules())
     return;
 
   auto *persistent_state = llvm::cast<ClangPersistentVariables>(
       target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
   if (!persistent_state)
     return;
 
+  std::shared_ptr<ClangModulesDeclVendor> decl_vendor =
+      persistent_state->GetClangModulesDeclVendor();
+  if (!decl_vendor)
+    return;
+
   StackFrame *frame = exe_ctx.GetFramePtr();
   if (!frame)
     return;
 
   Block *block = frame->GetFrameBlock();
   if (!block)
     return;
   SymbolContext sc;

lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

std::unique_ptr<Language::TypeScavenger> ObjCLanguage::GetTypeScavenger() {
 
   class ObjCModulesScavenger : public Language::TypeScavenger {
   protected:
     bool Find_Impl(ExecutionContextScope *exe_scope, const char *key,
                    ResultSet &results) override {
       bool result = false;
 
       if (auto *target = exe_scope->CalculateTarget().get()) {
-        if (auto *clang_modules_decl_vendor =
-                target->GetClangModulesDeclVendor()) {
+        auto *persistent_vars = llvm::cast<ClangPersistentVariables>(
+            target->GetPersistentExpressionStateForLanguage(
+                lldb::eLanguageTypeC));
+        if (std::shared_ptr<ClangModulesDeclVendor> clang_modules_decl_vendor =
+                persistent_vars->GetClangModulesDeclVendor()) {
           ConstString key_cs(key);
           auto types = clang_modules_decl_vendor->FindTypes(
               key_cs, /*max_matches*/ UINT32_MAX);
           if (!types.empty()) {
             result = true;
             std::unique_ptr<Language::TypeScavenger::Result> result(
                 new ObjCScavengerResult(types.front()));
             results.insert(std::move(result));

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

 
 char ScratchTypeSystemClang::ID;
 const llvm::NoneType ScratchTypeSystemClang::DefaultAST = llvm::None;
 
 ScratchTypeSystemClang::ScratchTypeSystemClang(Target &target,
                                                llvm::Triple triple)
     : TypeSystemClang("scratch ASTContext", triple), m_triple(triple),
       m_target_wp(target.shared_from_this()),
-      m_persistent_variables(new ClangPersistentVariables) {
+      m_persistent_variables(
+          new ClangPersistentVariables(target.shared_from_this())) {
   m_scratch_ast_source_up = CreateASTSource();
   m_scratch_ast_source_up->InstallASTContext(*this);
   llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> proxy_ast_source(
       m_scratch_ast_source_up->CreateProxy());
   SetExternalSource(proxy_ast_source);
 }
 
 void ScratchTypeSystemClang::Finalize() {

lldb/source/Target/CMakeLists.txt

add_lldb_library(lldbTarget
   LINK_LIBS
     lldbBreakpoint
     lldbCore
     lldbExpression
     lldbHost
     lldbInterpreter
     lldbSymbol
     lldbUtility
-    lldbPluginExpressionParserClang
     lldbPluginProcessUtility
 
   LINK_COMPONENTS
     Support
     MC
   )
 
 add_dependencies(lldbTarget
   LLDBTargetPropertiesGen
   LLDBTargetPropertiesEnumGen)

lldb/source/Target/Target.cpp

 }
 
 SourceManager &Target::GetSourceManager() {
   if (!m_source_manager_up)
     m_source_manager_up = std::make_unique<SourceManager>(shared_from_this());
   return *m_source_manager_up;
 }
 
-ClangModulesDeclVendor *Target::GetClangModulesDeclVendor() {
-  static std::mutex s_clang_modules_decl_vendor_mutex; // If this is contended
-                                                       // we can make it
-                                                       // per-target
-
-  {
-    std::lock_guard<std::mutex> guard(s_clang_modules_decl_vendor_mutex);
-
-    if (!m_clang_modules_decl_vendor_up) {
-      m_clang_modules_decl_vendor_up.reset(
-          ClangModulesDeclVendor::Create(*this));
-    }
-  }
-
-  return m_clang_modules_decl_vendor_up.get();
-}
-
 Target::StopHookSP Target::CreateStopHook(StopHook::StopHookKind kind) {
   lldb::user_id_t new_uid = ++m_stop_hook_next_id;
   Target::StopHookSP stop_hook_sp;
   switch (kind) {
   case StopHook::StopHookKind::CommandBased:
     stop_hook_sp.reset(new StopHookCommandLine(shared_from_this(), new_uid));
     break;
   case StopHook::StopHookKind::ScriptBased: