[clang] 367efa0 - [NFC] [Modules] Avoid scanning the stored decl list twice when replace

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-04-28T18:31:10+08:00
New Revision: 367efa0b0542e6f4171e8c914728946c302ab24b

URL: 
https://github.com/llvm/llvm-project/commit/367efa0b0542e6f4171e8c914728946c302ab24b
DIFF: 
https://github.com/llvm/llvm-project/commit/367efa0b0542e6f4171e8c914728946c302ab24b.diff

LOG: [NFC] [Modules] Avoid scanning the stored decl list twice when replace
external decls

This patch fixes a FIXME in StoredDeclList::replaceExternalDecls.

StoredDeclList::replaceExternalDecls will iterate the list first to
remove some declarations and iterate the list again to get the tail of
the list.

It should be better to avoid the second iterations.

Added: 
    

Modified: 
    clang/include/clang/AST/DeclContextInternals.h

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/DeclContextInternals.h 
b/clang/include/clang/AST/DeclContextInternals.h
index 42cc677f82135e..e169c485921929 100644
--- a/clang/include/clang/AST/DeclContextInternals.h
+++ b/clang/include/clang/AST/DeclContextInternals.h
@@ -42,11 +42,12 @@ class StoredDeclsList {
   /// external declarations.
   DeclsAndHasExternalTy Data;
 
-  template<typename Fn>
-  void erase_if(Fn ShouldErase) {
+  template <typename Fn> DeclListNode::Decls *erase_if(Fn ShouldErase) {
     Decls List = Data.getPointer();
+
     if (!List)
-      return;
+      return nullptr;
+
     ASTContext &C = getASTContext();
     DeclListNode::Decls NewHead = nullptr;
     DeclListNode::Decls *NewLast = nullptr;
@@ -79,6 +80,17 @@ class StoredDeclsList {
     Data.setPointer(NewHead);
 
     assert(llvm::none_of(getLookupResult(), ShouldErase) && "Still exists!");
+
+    if (!Data.getPointer())
+      // All declarations are erased.
+      return nullptr;
+    else if (NewHead.is<NamedDecl *>())
+      // The list only contains a declaration, the header itself.
+      return (DeclListNode::Decls *)&Data;
+    else {
+      assert(NewLast && NewLast->is<NamedDecl *>() && "Not the tail?");
+      return NewLast;
+    }
   }
 
   void erase(NamedDecl *ND) {
@@ -161,7 +173,7 @@ class StoredDeclsList {
   void replaceExternalDecls(ArrayRef<NamedDecl*> Decls) {
     // Remove all declarations that are either external or are replaced with
     // external declarations with higher visibilities.
-    erase_if([Decls](NamedDecl *ND) {
+    DeclListNode::Decls *Tail = erase_if([Decls](NamedDecl *ND) {
       if (ND->isFromASTFile())
         return true;
       // FIXME: Can we get rid of this loop completely?
@@ -189,24 +201,15 @@ class StoredDeclsList {
       DeclsAsList = Node;
     }
 
-    DeclListNode::Decls Head = Data.getPointer();
-    if (Head.isNull()) {
+    if (!Data.getPointer()) {
       Data.setPointer(DeclsAsList);
       return;
     }
 
-    // Find the end of the existing list.
-    // FIXME: It would be possible to preserve information from erase_if to
-    // avoid this rescan looking for the end of the list.
-    DeclListNode::Decls *Tail = &Head;
-    while (DeclListNode *Node = Tail->dyn_cast<DeclListNode *>())
-      Tail = &Node->Rest;
-
     // Append the Decls.
     DeclListNode *Node = C.AllocateDeclListNode(Tail->get<NamedDecl *>());
     Node->Rest = DeclsAsList;
     *Tail = Node;
-    Data.setPointer(Head);
   }
 
   /// Return the list of all the decls.


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits