From c4f6fbe971351273b19a4a819bf6ceae2b70b37e Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Thu, 23 Jan 2020 13:49:54 -0800 Subject: [PATCH] [lldb] Remove ClangASTImporter from Target Target is one of the classes responsible for vending ClangASTImporter. Target doesn't need to know anything about ClangASTImporter, so if we instead have ClangPersistentVariables vend it, we can preserve existing behavior while improving layering and removing dependencies from non-plugins to plugins. --- lldb/include/lldb/Symbol/TypeSystemClang.h | 3 ++- lldb/include/lldb/Target/Target.h | 3 --- .../ExpressionParser/Clang/ASTResultSynthesizer.cpp | 2 +- .../ExpressionParser/Clang/ClangPersistentVariables.cpp | 8 ++++++++ .../ExpressionParser/Clang/ClangPersistentVariables.h | 3 +++ .../Plugins/ExpressionParser/Clang/ClangUserExpression.cpp | 13 ++++++++++--- .../ExpressionParser/Clang/ClangUtilityFunction.cpp | 14 +++++++++++--- lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp | 12 +++++++++--- lldb/source/Symbol/TypeSystemClang.cpp | 2 +- lldb/source/Target/Target.cpp | 14 +------------- 10 files changed, 46 insertions(+), 28 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystemClang.h b/lldb/include/lldb/Symbol/TypeSystemClang.h index bb47ca4..9d4d5e5 100644 --- a/lldb/include/lldb/Symbol/TypeSystemClang.h +++ b/lldb/include/lldb/Symbol/TypeSystemClang.h @@ -26,6 +26,7 @@ #include "llvm/ADT/APSInt.h" #include "llvm/ADT/SmallVector.h" +#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h" #include "lldb/Core/ClangForward.h" #include "lldb/Expression/ExpressionVariable.h" #include "lldb/Symbol/CompilerType.h" @@ -1007,7 +1008,7 @@ public: PersistentExpressionState *GetPersistentExpressionState() override; private: lldb::TargetWP m_target_wp; - std::unique_ptr + std::unique_ptr m_persistent_variables; // These are the persistent variables associated // with this process for the expression parser std::unique_ptr m_scratch_ast_source_up; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 4396f05..414a669 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1056,8 +1056,6 @@ public: const char *name, Status &error); - lldb::ClangASTImporterSP GetClangASTImporter(); - // Install any files through the platform that need be to installed prior to // launching or attaching. Status Install(ProcessLaunchInfo *launch_info); @@ -1304,7 +1302,6 @@ protected: typedef std::map REPLMap; REPLMap m_repl_map; - lldb::ClangASTImporterSP m_ast_importer_sp; lldb::ClangModulesDeclVendorUP m_clang_modules_decl_vendor_up; lldb::SourceManagerUP m_source_manager_up; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp index f7d4f17..7b2acb4 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp @@ -459,7 +459,7 @@ void ASTResultSynthesizer::CommitPersistentDecls() { StringRef name = decl->getName(); ConstString name_cs(name.str().c_str()); - Decl *D_scratch = m_target.GetClangASTImporter()->DeportDecl( + Decl *D_scratch = persistent_vars->GetClangASTImporter()->DeportDecl( &scratch_ctx->getASTContext(), decl); if (!D_scratch) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp index e3ada1c..f847dc9 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp @@ -9,6 +9,7 @@ #include "ClangPersistentVariables.h" #include "lldb/Core/Value.h" +#include "lldb/Symbol/ClangASTImporter.h" #include "lldb/Symbol/TypeSystemClang.h" #include "lldb/Target/Target.h" #include "lldb/Utility/DataExtractor.h" @@ -99,3 +100,10 @@ clang::NamedDecl * ClangPersistentVariables::GetPersistentDecl(ConstString name) { return m_persistent_decls.lookup(name.GetCString()).m_decl; } + +lldb::ClangASTImporterSP ClangPersistentVariables::GetClangASTImporter() { + if (!m_ast_importer_sp) { + m_ast_importer_sp = std::make_shared(); + } + return m_ast_importer_sp; +} diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h index 2ebe1c7..b72e43a 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h @@ -36,6 +36,8 @@ public: return pv->getKind() == PersistentExpressionState::eKindClang; } + lldb::ClangASTImporterSP GetClangASTImporter(); + lldb::ExpressionVariableSP CreatePersistentVariable(const lldb::ValueObjectSP &valobj_sp) override; @@ -96,6 +98,7 @@ private: m_hand_loaded_clang_modules; ///< These are Clang modules we hand-loaded; ///these are the highest- ///< priority source for macros. + lldb::ClangASTImporterSP m_ast_importer_sp; }; } // namespace lldb_private diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index 06c3ea9..2fca504 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -896,9 +896,16 @@ void ClangUserExpression::ClangUserExpressionHelper::ResetDeclMap( Materializer::PersistentVariableDelegate &delegate, bool keep_result_in_memory, ValueObject *ctx_obj) { - m_expr_decl_map_up.reset(new ClangExpressionDeclMap( - keep_result_in_memory, &delegate, exe_ctx.GetTargetSP(), - exe_ctx.GetTargetRef().GetClangASTImporter(), ctx_obj)); + lldb::ClangASTImporterSP ast_importer; + auto *state = exe_ctx.GetTargetSP()->GetPersistentExpressionStateForLanguage( + lldb::eLanguageTypeC); + if (state) { + auto *persistent_vars = llvm::cast(state); + ast_importer = persistent_vars->GetClangASTImporter(); + } + m_expr_decl_map_up.reset( + new ClangExpressionDeclMap(keep_result_in_memory, &delegate, + exe_ctx.GetTargetSP(), ast_importer, ctx_obj)); } clang::ASTConsumer * diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp index 010c8d9..5e97e95 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp @@ -12,6 +12,7 @@ #include "ClangExpressionDeclMap.h" #include "ClangExpressionParser.h" #include "ClangExpressionSourceCode.h" +#include "ClangPersistentVariables.h" #include #if HAVE_SYS_TYPES_H @@ -159,7 +160,14 @@ bool ClangUtilityFunction::Install(DiagnosticManager &diagnostic_manager, void ClangUtilityFunction::ClangUtilityFunctionHelper::ResetDeclMap( ExecutionContext &exe_ctx, bool keep_result_in_memory) { - m_expr_decl_map_up.reset(new ClangExpressionDeclMap( - keep_result_in_memory, nullptr, exe_ctx.GetTargetSP(), - exe_ctx.GetTargetRef().GetClangASTImporter(), nullptr)); + lldb::ClangASTImporterSP ast_importer; + auto *state = exe_ctx.GetTargetSP()->GetPersistentExpressionStateForLanguage( + lldb::eLanguageTypeC); + if (state) { + auto *persistent_vars = llvm::cast(state); + ast_importer = persistent_vars->GetClangASTImporter(); + } + m_expr_decl_map_up.reset( + new ClangExpressionDeclMap(keep_result_in_memory, nullptr, + exe_ctx.GetTargetSP(), ast_importer, nullptr)); } diff --git a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp index 986e7997..43b2cad 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp @@ -8,14 +8,14 @@ #include "BlockPointer.h" +#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h" #include "lldb/Core/ValueObject.h" #include "lldb/DataFormatters/FormattersHelpers.h" -#include "lldb/Symbol/TypeSystemClang.h" #include "lldb/Symbol/ClangASTImporter.h" #include "lldb/Symbol/CompilerType.h" #include "lldb/Symbol/TypeSystem.h" +#include "lldb/Symbol/TypeSystemClang.h" #include "lldb/Target/Target.h" - #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/Log.h" @@ -56,7 +56,13 @@ public: return; } - ClangASTImporterSP clang_ast_importer = target_sp->GetClangASTImporter(); + lldb::ClangASTImporterSP clang_ast_importer; + auto *state = target_sp->GetPersistentExpressionStateForLanguage( + lldb::eLanguageTypeC_plus_plus); + if (state) { + auto *persistent_vars = llvm::cast(state); + clang_ast_importer = persistent_vars->GetClangASTImporter(); + } if (!clang_ast_importer) { return; diff --git a/lldb/source/Symbol/TypeSystemClang.cpp b/lldb/source/Symbol/TypeSystemClang.cpp index 174b521..7a49fba 100644 --- a/lldb/source/Symbol/TypeSystemClang.cpp +++ b/lldb/source/Symbol/TypeSystemClang.cpp @@ -9262,7 +9262,7 @@ TypeSystemClangForExpressions::TypeSystemClangForExpressions( m_target_wp(target.shared_from_this()), m_persistent_variables(new ClangPersistentVariables) { m_scratch_ast_source_up.reset(new ClangASTSource( - target.shared_from_this(), target.GetClangASTImporter())); + target.shared_from_this(), m_persistent_variables->GetClangASTImporter())); m_scratch_ast_source_up->InstallASTContext(*this); llvm::IntrusiveRefCntPtr proxy_ast_source( m_scratch_ast_source_up->CreateProxy()); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 7cfe81d..64913f9 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -37,7 +37,6 @@ #include "lldb/Interpreter/OptionGroupWatchpoint.h" #include "lldb/Interpreter/OptionValues.h" #include "lldb/Interpreter/Property.h" -#include "lldb/Symbol/ClangASTImporter.h" #include "lldb/Symbol/Function.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Symbol/Symbol.h" @@ -91,7 +90,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, m_mutex(), m_arch(target_arch), m_images(this), m_section_load_history(), m_breakpoint_list(false), m_internal_breakpoint_list(true), m_watchpoint_list(), m_process_sp(), m_search_filter_sp(), - m_image_search_paths(ImageSearchPathsChanged, this), m_ast_importer_sp(), + m_image_search_paths(ImageSearchPathsChanged, this), m_source_manager_up(), m_stop_hooks(), m_stop_hook_next_id(0), m_valid(true), m_suppress_stop_hooks(false), m_is_dummy_target(is_dummy_target), @@ -1378,7 +1377,6 @@ void Target::ClearModules(bool delete_locations) { m_section_load_history.Clear(); m_images.Clear(); m_scratch_type_system_map.Clear(); - m_ast_importer_sp.reset(); } void Target::DidExec() { @@ -2258,16 +2256,6 @@ Target::GetUtilityFunctionForLanguage(const char *text, return utility_fn; } -ClangASTImporterSP Target::GetClangASTImporter() { - if (m_valid) { - if (!m_ast_importer_sp) { - m_ast_importer_sp = std::make_shared(); - } - return m_ast_importer_sp; - } - return ClangASTImporterSP(); -} - void Target::SettingsInitialize() { Process::SettingsInitialize(); } void Target::SettingsTerminate() { Process::SettingsTerminate(); } -- 2.7.4