teemperor created this revision.
teemperor added reviewers: shafik, aprantl.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
teemperor edited the summary of this revision.
Herald added a subscriber: rnkovacs.
The ASTImporterDelegate is currently responsible for both recording and also
completing
types. This patch moves the actual completion and recording code outside the
ASTImporterDelegate
to reduce the amount of responsibilities the ASTImporterDelegate has to fulfill.
As I anyway had to touch the code when moving I also documented and refactored
most of it
(e.g. no more asserts that we call the deporting start/end function always as a
pair).
Note that I had to make the ASTImporterDelegate and it's related functions
public now so that
I can move out the functionality in another class (that doesn't need to be in
the header).
Repository:
rLLDB LLDB
https://reviews.llvm.org/D61478
Files:
lldb/include/lldb/Symbol/ClangASTImporter.h
lldb/source/Symbol/ClangASTImporter.cpp
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<NamedDecl *> m_decls_to_deport;
+ std::set<NamedDecl *> 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<TagDecl>(decl)) {
+ if (auto *original_tag_decl = dyn_cast<TagDecl>(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<ObjCContainerDecl>(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<TagDecl>(to) && !isa<ObjCInterfaceDecl>(to))
+ return;
+ RecordDecl *from_record_decl = dyn_cast<RecordDecl>(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<NamedDecl>(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<void *>(src_ctx),
static_cast<void *>(dst_ctx));
- ImporterDelegateSP delegate_sp(GetDelegate(dst_ctx, src_ctx));
-
- if (!delegate_sp)
- return nullptr;
-
- std::set<NamedDecl *> decls_to_deport;
- std::set<NamedDecl *> decls_already_deported;
-
DeclContextOverride decl_context_override;
- if (const clang::TagType *tag_type =
- clang::QualType::getFromOpaquePtr(type)->getAs<TagType>()) {
- 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<TagType>())
+ 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<void *>(decl),
static_cast<void *>(src_ctx), static_cast<void *>(dst_ctx));
- ImporterDelegateSP delegate_sp(GetDelegate(dst_ctx, src_ctx));
-
- if (!delegate_sp)
- return nullptr;
-
- std::set<NamedDecl *> decls_to_deport;
- std::set<NamedDecl *> 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<clang::NamedDecl *> *decls_to_deport,
- std::set<clang::NamedDecl *> *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<TagDecl>(decl)) {
- if (TagDecl *original_tag_decl = dyn_cast<TagDecl>(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<ObjCContainerDecl>(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<void *>(&from->getASTContext()),
static_cast<void *>(&to->getASTContext()));
} else {
- if (m_decls_to_deport && m_decls_already_deported) {
- if (isa<TagDecl>(to) || isa<ObjCInterfaceDecl>(to)) {
- RecordDecl *from_record_decl = dyn_cast<RecordDecl>(from);
- if (from_record_decl == nullptr ||
- !from_record_decl->isInjectedClassName()) {
- NamedDecl *to_named_decl = dyn_cast<NamedDecl>(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) {
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<const clang::Decl *, DeclOrigin> 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<clang::Decl *> 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<clang::NamedDecl *> *decls_to_deport,
- std::set<clang::NamedDecl *> *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<clang::Decl *> 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<clang::Decl *> m_decls_to_ignore;
- std::set<clang::NamedDecl *> *m_decls_to_deport;
- std::set<clang::NamedDecl *> *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<ASTImporterDelegate> ImporterDelegateSP;
@@ -387,6 +382,7 @@
}
}
+public:
DeclOrigin GetDeclOrigin(const clang::Decl *decl);
clang::FileManager m_file_manager;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
