[Lldb-commits] [lldb] r372385 - Move decl completion out of the ASTImporterDelegate and document it [NFC]
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 20 05:52:55 PDT 2019
Author: teemperor
Date: Fri Sep 20 05:52:55 2019
New Revision: 372385
URL: http://llvm.org/viewvc/llvm-project?rev=372385&view=rev
Log:
Move decl completion out of the ASTImporterDelegate and document it [NFC]
Summary:
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).
Reviewers: shafik, aprantl, martong, a.sidorin
Reviewed By: martong
Subscribers: rnkovacs, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D61478
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
lldb/trunk/source/Symbol/ClangASTImporter.cpp
Modified: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTImporter.h?rev=372385&r1=372384&r2=372385&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h Fri Sep 20 05:52:55 2019
@@ -210,7 +210,7 @@ public:
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 @@ private:
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) {
setODRHandling(clang::ASTImporter::ODRHandlingType::Liberal);
}
@@ -287,43 +293,32 @@ private:
}
};
- 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;
@@ -389,6 +384,7 @@ private:
}
}
+public:
DeclOrigin GetDeclOrigin(const clang::Decl *decl);
clang::FileManager m_file_manager;
Modified: lldb/trunk/source/Symbol/ClangASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTImporter.cpp?rev=372385&r1=372384&r2=372385&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp Fri Sep 20 05:52:55 2019
@@ -250,6 +250,94 @@ public:
}
};
+namespace {
+/// Completes all imported TagDecls at the end of the scope.
+///
+/// While in a CompleteTagDeclsScope, every decl that could be completed will
+/// be completed at the end of the scope (including all Decls that are
+/// imported while completing the original Decls).
+class CompleteTagDeclsScope : 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_complete;
+ std::set<NamedDecl *> m_decls_already_completed;
+ clang::ASTContext *m_dst_ctx;
+ clang::ASTContext *m_src_ctx;
+ ClangASTImporter &importer;
+
+public:
+ /// Constructs a CompleteTagDeclsScope.
+ /// \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 CompleteTagDeclsScope(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 ~CompleteTagDeclsScope() {
+ ClangASTImporter::ASTContextMetadataSP to_context_md =
+ importer.GetContextMetadata(m_dst_ctx);
+
+ // Complete all decls we collected until now.
+ while (!m_decls_to_complete.empty()) {
+ NamedDecl *decl = *m_decls_to_complete.begin();
+
+ m_decls_already_completed.insert(decl);
+ m_decls_to_complete.erase(decl);
+
+ // We should only complete decls coming from the source context.
+ 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 complete 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 completed this type.
+ if (m_decls_already_completed.count(to_named_decl) != 0)
+ return;
+ m_decls_to_complete.insert(to_named_decl);
+ }
+};
+} // namespace
+
lldb::opaque_compiler_type_t
ClangASTImporter::DeportType(clang::ASTContext *dst_ctx,
clang::ASTContext *src_ctx,
@@ -263,27 +351,16 @@ ClangASTImporter::DeportType(clang::ASTC
(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);
+ if (auto *t = QualType::getFromOpaquePtr(type)->getAs<TagType>())
+ decl_context_override.OverrideAllDeclsFromContainingFunction(t->getDecl());
- lldb::opaque_compiler_type_t result = CopyType(dst_ctx, src_ctx, type);
-
- delegate_sp->ExecuteDeportWorkQueues();
+ lldb::opaque_compiler_type_t result;
+ {
+ CompleteTagDeclsScope complete_scope(*this, dst_ctx, src_ctx);
+ result = CopyType(dst_ctx, src_ctx, type);
+ }
if (!result)
return nullptr;
@@ -302,23 +379,15 @@ clang::Decl *ClangASTImporter::DeportDec
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;
+ {
+ CompleteTagDeclsScope complete_scope(*this, dst_ctx, src_ctx);
+ result = CopyDecl(dst_ctx, src_ctx, decl);
+ }
if (!result)
return nullptr;
@@ -870,63 +939,6 @@ ClangASTImporter::ASTImporterDelegate::I
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);
@@ -1092,18 +1104,8 @@ void ClangASTImporter::ASTImporterDelega
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) {
More information about the lldb-commits
mailing list