Index: lldb/include/lldb/Symbol/ClangASTImporter.h =================================================================== --- lldb/include/lldb/Symbol/ClangASTImporter.h +++ lldb/include/lldb/Symbol/ClangASTImporter.h @@ -210,7 +210,7 @@ void ForgetDestination(clang::ASTContext *dst_ctx); void ForgetSource(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx); -private: +public: struct DeclOrigin { DeclOrigin() : ctx(nullptr), decl(nullptr) {} @@ -235,23 +235,29 @@ typedef std::map OriginMap; + /// Listener interface used by the ASTImporterDelegate to inform other code + /// about decls that have been imported the first time. + struct NewDeclListener { + virtual ~NewDeclListener() = default; + /// A decl has been imported for the first time. + virtual void NewDeclImported(clang::Decl *from, clang::Decl *to) = 0; + }; + /// ASTImporter that intercepts and records the import process of the /// underlying ASTImporter. /// /// This class updates the map from declarations to their original - /// declarations and can record and complete declarations that have been - /// imported in a certain interval. + /// declarations and can record declarations that have been imported in a + /// certain interval. /// /// When intercepting a declaration import, the ASTImporterDelegate uses the /// CxxModuleHandler to replace any missing or malformed declarations with /// their counterpart from a C++ module. - class ASTImporterDelegate : public clang::ASTImporter { - public: + struct ASTImporterDelegate : public clang::ASTImporter { ASTImporterDelegate(ClangASTImporter &master, clang::ASTContext *target_ctx, clang::ASTContext *source_ctx) : clang::ASTImporter(*target_ctx, master.m_file_manager, *source_ctx, master.m_file_manager, true /*minimal*/), - m_decls_to_deport(nullptr), m_decls_already_deported(nullptr), m_master(master), m_source_ctx(source_ctx) {} /// Scope guard that attaches a CxxModuleHandler to an ASTImporterDelegate @@ -285,43 +291,32 @@ } }; - protected: - llvm::Expected ImportImpl(clang::Decl *From) override; - - public: - // A call to "InitDeportWorkQueues" puts the delegate into deport mode. - // In deport mode, every copied Decl that could require completion is - // recorded and placed into the decls_to_deport set. - // - // A call to "ExecuteDeportWorkQueues" completes all the Decls that - // are in decls_to_deport, adding any Decls it sees along the way that it - // hasn't already deported. It proceeds until decls_to_deport is empty. - // - // These calls must be paired. Leaving a delegate in deport mode or trying - // to start deport delegate with a new pair of queues will result in an - // assertion failure. - - void - InitDeportWorkQueues(std::set *decls_to_deport, - std::set *decls_already_deported); - void ExecuteDeportWorkQueues(); - void ImportDefinitionTo(clang::Decl *to, clang::Decl *from); void Imported(clang::Decl *from, clang::Decl *to) override; clang::Decl *GetOriginalDecl(clang::Decl *To) override; + void SetImportListener(NewDeclListener *listener) { + assert(m_new_decl_listener == nullptr && "Already attached a listener?"); + m_new_decl_listener = listener; + } + void RemoveImportListener() { m_new_decl_listener = nullptr; } + + protected: + llvm::Expected ImportImpl(clang::Decl *From) override; + + private: /// Decls we should ignore when mapping decls back to their original /// ASTContext. Used by the CxxModuleHandler to mark declarations that /// were created from the 'std' C++ module to prevent that the Importer /// tries to sync them with the broken equivalent in the debug info AST. std::set m_decls_to_ignore; - std::set *m_decls_to_deport; - std::set *m_decls_already_deported; ClangASTImporter &m_master; clang::ASTContext *m_source_ctx; CxxModuleHandler *m_std_handler = nullptr; + /// The currently attached listener. + NewDeclListener *m_new_decl_listener = nullptr; }; typedef std::shared_ptr ImporterDelegateSP; @@ -387,6 +382,7 @@ } } +public: DeclOrigin GetDeclOrigin(const clang::Decl *decl); clang::FileManager m_file_manager; Index: lldb/source/Symbol/ClangASTImporter.cpp =================================================================== --- lldb/source/Symbol/ClangASTImporter.cpp +++ lldb/source/Symbol/ClangASTImporter.cpp @@ -241,6 +241,95 @@ } }; +namespace { +/// Puts the ClangASTImporter into deport mode in this scope. +/// +/// In deport mode, every copied Decl that could require completion will +/// be completed at the end of the scope (including all Decls that are +/// imported while completing the original Decls). +class DeportQueueScope : public ClangASTImporter::NewDeclListener { + ClangASTImporter::ImporterDelegateSP m_delegate; + // FIXME: Investigate how many decls we usually have in these sets and + // see if we can use SmallPtrSet instead here. + std::set m_decls_to_deport; + std::set m_decls_already_deported; + clang::ASTContext *m_dst_ctx; + clang::ASTContext *m_src_ctx; + ClangASTImporter &importer; + +public: + /// Constructs a DeportQueueScope. + /// \param importer The ClangASTImporter that we should observe. + /// \param dst_ctx The ASTContext to which Decls are imported. + /// \param src_ctx The ASTContext from which Decls are imported. + explicit DeportQueueScope(ClangASTImporter &importer, + clang::ASTContext *dst_ctx, + clang::ASTContext *src_ctx) + : m_delegate(importer.GetDelegate(dst_ctx, src_ctx)), m_dst_ctx(dst_ctx), + m_src_ctx(src_ctx), importer(importer) { + m_delegate->SetImportListener(this); + } + + virtual ~DeportQueueScope() { + ClangASTImporter::ASTContextMetadataSP to_context_md = + importer.GetContextMetadata(m_dst_ctx); + + // Deport all decls we collected until now. + while (!m_decls_to_deport.empty()) { + NamedDecl *decl = *m_decls_to_deport.begin(); + + m_decls_already_deported.insert(decl); + m_decls_to_deport.erase(decl); + + // Otherwise we should never have added this because it doesn't need to + // be deported. + assert(to_context_md->m_origins[decl].ctx == m_src_ctx); + + Decl *original_decl = to_context_md->m_origins[decl].decl; + + // Complete the decl now. + ClangASTContext::GetCompleteDecl(m_src_ctx, original_decl); + if (auto *tag_decl = dyn_cast(decl)) { + if (auto *original_tag_decl = dyn_cast(original_decl)) { + if (original_tag_decl->isCompleteDefinition()) { + m_delegate->ImportDefinitionTo(tag_decl, original_tag_decl); + tag_decl->setCompleteDefinition(true); + } + } + + tag_decl->setHasExternalLexicalStorage(false); + tag_decl->setHasExternalVisibleStorage(false); + } else if (auto *container_decl = dyn_cast(decl)) { + container_decl->setHasExternalLexicalStorage(false); + container_decl->setHasExternalVisibleStorage(false); + } + + to_context_md->m_origins.erase(decl); + } + + // Stop listening to imported decls. We do this after clearing the + // Decls we needed to import to catch all Decls they might have pulled in. + m_delegate->RemoveImportListener(); + } + + void NewDeclImported(clang::Decl *from, clang::Decl *to) override { + // Filter out decls that we can't complete later. + if (!isa(to) && !isa(to)) + return; + RecordDecl *from_record_decl = dyn_cast(from); + // We don't need to deport injected class name decls. + if (from_record_decl && from_record_decl->isInjectedClassName()) + return; + + NamedDecl *to_named_decl = dyn_cast(to); + // Check if we already deported this type. + if (m_decls_already_deported.count(to_named_decl) != 0) + return; + m_decls_to_deport.insert(to_named_decl); + } +}; +} // namespace + lldb::opaque_compiler_type_t ClangASTImporter::DeportType(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx, @@ -254,27 +343,16 @@ (unsigned long long)type, static_cast(src_ctx), static_cast(dst_ctx)); - ImporterDelegateSP delegate_sp(GetDelegate(dst_ctx, src_ctx)); - - if (!delegate_sp) - return nullptr; - - std::set decls_to_deport; - std::set decls_already_deported; - DeclContextOverride decl_context_override; - if (const clang::TagType *tag_type = - clang::QualType::getFromOpaquePtr(type)->getAs()) { - decl_context_override.OverrideAllDeclsFromContainingFunction( - tag_type->getDecl()); - } - - delegate_sp->InitDeportWorkQueues(&decls_to_deport, &decls_already_deported); - - lldb::opaque_compiler_type_t result = CopyType(dst_ctx, src_ctx, type); + if (auto *t = QualType::getFromOpaquePtr(type)->getAs()) + decl_context_override.OverrideAllDeclsFromContainingFunction(t->getDecl()); - delegate_sp->ExecuteDeportWorkQueues(); + lldb::opaque_compiler_type_t result; + { + DeportQueueScope deport_scope(*this, dst_ctx, src_ctx); + result = CopyType(dst_ctx, src_ctx, type); + } if (!result) return nullptr; @@ -293,23 +371,15 @@ decl->getDeclKindName(), static_cast(decl), static_cast(src_ctx), static_cast(dst_ctx)); - ImporterDelegateSP delegate_sp(GetDelegate(dst_ctx, src_ctx)); - - if (!delegate_sp) - return nullptr; - - std::set decls_to_deport; - std::set decls_already_deported; - DeclContextOverride decl_context_override; decl_context_override.OverrideAllDeclsFromContainingFunction(decl); - delegate_sp->InitDeportWorkQueues(&decls_to_deport, &decls_already_deported); - - clang::Decl *result = CopyDecl(dst_ctx, src_ctx, decl); - - delegate_sp->ExecuteDeportWorkQueues(); + clang::Decl *result; + { + DeportQueueScope deport_scope(*this, dst_ctx, src_ctx); + result = CopyDecl(dst_ctx, src_ctx, decl); + } if (!result) return nullptr; @@ -848,63 +918,6 @@ return ASTImporter::ImportImpl(From); } -void ClangASTImporter::ASTImporterDelegate::InitDeportWorkQueues( - std::set *decls_to_deport, - std::set *decls_already_deported) { - assert(!m_decls_to_deport); - assert(!m_decls_already_deported); - - m_decls_to_deport = decls_to_deport; - m_decls_already_deported = decls_already_deported; -} - -void ClangASTImporter::ASTImporterDelegate::ExecuteDeportWorkQueues() { - assert(m_decls_to_deport); - assert(m_decls_already_deported); - - ASTContextMetadataSP to_context_md = - m_master.GetContextMetadata(&getToContext()); - - while (!m_decls_to_deport->empty()) { - NamedDecl *decl = *m_decls_to_deport->begin(); - - m_decls_already_deported->insert(decl); - m_decls_to_deport->erase(decl); - - DeclOrigin &origin = to_context_md->m_origins[decl]; - UNUSED_IF_ASSERT_DISABLED(origin); - - assert(origin.ctx == - m_source_ctx); // otherwise we should never have added this - // because it doesn't need to be deported - - Decl *original_decl = to_context_md->m_origins[decl].decl; - - ClangASTContext::GetCompleteDecl(m_source_ctx, original_decl); - - if (TagDecl *tag_decl = dyn_cast(decl)) { - if (TagDecl *original_tag_decl = dyn_cast(original_decl)) { - if (original_tag_decl->isCompleteDefinition()) { - ImportDefinitionTo(tag_decl, original_tag_decl); - tag_decl->setCompleteDefinition(true); - } - } - - tag_decl->setHasExternalLexicalStorage(false); - tag_decl->setHasExternalVisibleStorage(false); - } else if (ObjCContainerDecl *container_decl = - dyn_cast(decl)) { - container_decl->setHasExternalLexicalStorage(false); - container_decl->setHasExternalVisibleStorage(false); - } - - to_context_md->m_origins.erase(decl); - } - - m_decls_to_deport = nullptr; - m_decls_already_deported = nullptr; -} - void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo( clang::Decl *to, clang::Decl *from) { ASTImporter::Imported(from, to); @@ -1036,18 +1049,8 @@ static_cast(&from->getASTContext()), static_cast(&to->getASTContext())); } else { - if (m_decls_to_deport && m_decls_already_deported) { - if (isa(to) || isa(to)) { - RecordDecl *from_record_decl = dyn_cast(from); - if (from_record_decl == nullptr || - !from_record_decl->isInjectedClassName()) { - NamedDecl *to_named_decl = dyn_cast(to); - - if (!m_decls_already_deported->count(to_named_decl)) - m_decls_to_deport->insert(to_named_decl); - } - } - } + if (m_new_decl_listener) + m_new_decl_listener->NewDeclImported(from, to); if (to_context_md->m_origins.find(to) == to_context_md->m_origins.end() || user_id != LLDB_INVALID_UID) {