[PATCH] D69933: [ASTImporter] Limit imports of structs

[Jaroslav Sevcik via Phabricator via cfe-commits]

jarin updated this revision to Diff 239235.
jarin added a comment.
Herald added a subscriber: lldb-commits.

I changed the diff so that it does not touch Clang's AST importer, instead it patches LLDB's wrapper of the AST importer.

The idea is to only import complete a record if the current evaluation asked for them to be completed. In particular, if the current evaluation has not ask for a record R to be completed and LLDB is being asked to import R, we supply an incomplete version of R even if we happen to have a complete definition of R lying around in parsed debug info.

This is achieved in a hacky way - if we have a complete record R, we pretend it is incomplete by temporarily clearing R's isCompleteDefinition bit. Interestingly, this hack is already used in ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69933/new/

https://reviews.llvm.org/D69933

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
@@ -42,9 +42,12 @@
   if (!delegate_sp)
     return CompilerType();
 
+  delegate_sp->SetCompleted(src_qual_type->getAsTagDecl());
+
   ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp, &dst_clang_ast);
 
   llvm::Expected<QualType> ret_or_error = delegate_sp->Import(src_qual_type);
+
   if (!ret_or_error) {
     Log *log =
       lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
@@ -252,6 +255,7 @@
       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->SetCompleted(original_tag_decl);
             m_delegate->ImportDefinitionTo(tag_decl, original_tag_decl);
             tag_decl->setCompleteDefinition(true);
           }
@@ -584,6 +588,8 @@
   ImporterDelegateSP delegate_sp(
       GetDelegate(&decl->getASTContext(), decl_origin.ctx));
 
+  delegate_sp->SetCompleted(decl_origin.decl);
+
   ASTImporterDelegate::CxxModuleScope std_scope(*delegate_sp,
                                                 &decl->getASTContext());
   if (delegate_sp)
@@ -855,6 +861,10 @@
 
 ClangASTImporter::MapCompleter::~MapCompleter() { return; }
 
+void ClangASTImporter::ASTImporterDelegate::SetCompleted(Decl *from) {
+  m_completed_decls.insert(from);
+}
+
 llvm::Expected<Decl *>
 ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) {
   if (m_std_handler) {
@@ -903,7 +913,20 @@
     }
   }
 
-  return ASTImporter::ImportImpl(From);
+  CXXRecordDecl *record_decl_to_set_complete = nullptr;
+  if (CXXRecordDecl *record_decl = dyn_cast<CXXRecordDecl>(From)) {
+    if (record_decl->isCompleteDefinition() &&
+        !record_decl->isAnonymousStructOrUnion() &&
+        m_completed_decls.find(From) == m_completed_decls.end()) {
+      record_decl->setCompleteDefinition(false);
+      record_decl_to_set_complete = record_decl;
+    }
+  }
+  llvm::Expected<Decl *> result = ASTImporter::ImportImpl(From);
+  if (record_decl_to_set_complete) {
+    record_decl_to_set_complete->setCompleteDefinition(true);
+  }
+  return result;
 }
 
 void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===================================================================
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -231,6 +231,8 @@
 
     clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
+    void SetCompleted(clang::Decl *from);
+
     void SetImportListener(NewDeclListener *listener) {
       assert(m_new_decl_listener == nullptr && "Already attached a listener?");
       m_new_decl_listener = listener;
@@ -246,6 +248,7 @@
     /// 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.
     llvm::SmallPtrSet<clang::Decl *, 16> m_decls_to_ignore;
+    llvm::SmallPtrSet<clang::Decl *, 16> m_completed_decls;
     ClangASTImporter &m_master;
     clang::ASTContext *m_source_ctx;
     CxxModuleHandler *m_std_handler = nullptr;
@@ -257,6 +260,7 @@
   typedef llvm::DenseMap<clang::ASTContext *, ImporterDelegateSP> DelegateMap;
   typedef llvm::DenseMap<const clang::NamespaceDecl *, NamespaceMapSP>
       NamespaceMetaMap;
+  typedef std::set<const clang::Decl *> CompletedRecordSet;
 
   struct ASTContextMetadata {
     ASTContextMetadata(clang::ASTContext *dst_ctx)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69933.239235.patch
Type: text/x-patch
Size: 3679 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200121/cc5e7949/attachment-0001.bin>