[clang] First run at removing the linked-list for decls. Tests not run, but … (PR #113983)

via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 28 17:04:55 PDT 2024


github-actions[bot] wrote:

<!--LLVM CODE FORMAT COMMENT: {clang-format}-->


:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

``````````bash
git-clang-format --diff 19131c7f36e047898ea954ee5a187ac62f2ab09b 09c1c890394ecaa66b20cd933ba0d85c2141e34f --extensions h,cpp -- clang/include/clang/AST/Decl.h clang/include/clang/AST/DeclBase.h clang/include/clang/AST/DeclCXX.h clang/lib/AST/Decl.cpp clang/lib/AST/DeclBase.cpp clang/lib/AST/DeclPrinter.cpp
``````````

</details>

<details>
<summary>
View the diff from clang-format here.
</summary>

``````````diff
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 3c53658ecd..f58f4f21bb 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -398,7 +398,8 @@ protected:
         TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
         IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
         CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)),
-        ModuleOwnership(static_cast<unsigned>(getModuleOwnershipKindForChildOf(DC))) {
+        ModuleOwnership(
+            static_cast<unsigned>(getModuleOwnershipKindForChildOf(DC))) {
     if (StatisticsEnabled) add(DK);
   }
 
@@ -448,9 +449,10 @@ public:
   Kind getKind() const { return static_cast<Kind>(DeclKind); }
   const char *getDeclKindName() const;
 
-  //TODO: ERICH: figure out how necessary these are? Can be implemented 'dumb-ly'
-  //Decl *getNextDeclInContext() { return NextInContextAndBits.getPointer(); }
-  //const Decl *getNextDeclInContext() const {return NextInContextAndBits.getPointer();}
+  // TODO: ERICH: figure out how necessary these are? Can be implemented
+  // 'dumb-ly' Decl *getNextDeclInContext() { return
+  // NextInContextAndBits.getPointer(); } const Decl *getNextDeclInContext()
+  // const {return NextInContextAndBits.getPointer();}
 
   DeclContext *getDeclContext() {
     if (isInSemaDC())
@@ -2060,21 +2062,21 @@ protected:
 
   /// FirstDecl - The first declaration stored within this declaration
   /// context.
-  ///mutable Decl *FirstDecl = nullptr;
+  /// mutable Decl *FirstDecl = nullptr;
 
   /// LastDecl - The last declaration stored within this declaration
   /// context. FIXME: We could probably cache this value somewhere
   /// outside of the DeclContext, to reduce the size of DeclContext by
   /// another pointer.
-  ///mutable Decl *LastDecl = nullptr;
+  /// mutable Decl *LastDecl = nullptr;
 
   /// Build up a chain of declarations.
   ///
   /// \returns the first/last pair of declarations.
   // TODO: ERICH: this ends up being just a filter if it doesn't need to update
   // next decl.
-  //static std::pair<Decl *, Decl *>
-  //BuildDeclChain(ArrayRef<Decl*> Decls, bool FieldsAlreadyLoaded);
+  // static std::pair<Decl *, Decl *>
+  // BuildDeclChain(ArrayRef<Decl*> Decls, bool FieldsAlreadyLoaded);
 
   DeclContext(Decl::Kind K);
 
@@ -2308,63 +2310,62 @@ public:
   void collectAllContexts(SmallVectorImpl<DeclContext *> &Contexts);
 
   // TODO: ERICH: Remove
-/*
-  /// decl_iterator - Iterates through the declarations stored
-  /// within this context.
-  class decl_iterator {
-    /// Current - The current declaration.
-    Decl *Current = nullptr;
-
-  public:
-    using value_type = Decl *;
-    using reference = const value_type &;
-    using pointer = const value_type *;
-    using iterator_category = std::forward_iterator_tag;
-    using difference_type = std::ptrdiff_t;
-
-    decl_iterator() = default;
-    explicit decl_iterator(Decl *C) : Current(C) {}
-
-    reference operator*() const { return Current; }
-
-    // This doesn't meet the iterator requirements, but it's convenient
-    value_type operator->() const { return Current; }
-
-    decl_iterator& operator++() {
-      Current = Current->getNextDeclInContext();
-      return *this;
-    }
-
-    decl_iterator operator++(int) {
-      decl_iterator tmp(*this);
-      ++(*this);
-      return tmp;
-    }
-
-    friend bool operator==(decl_iterator x, decl_iterator y) {
-      return x.Current == y.Current;
-    }
-
-    friend bool operator!=(decl_iterator x, decl_iterator y) {
-      return x.Current != y.Current;
-    }
-  };
-  */
+  /*
+    /// decl_iterator - Iterates through the declarations stored
+    /// within this context.
+    class decl_iterator {
+      /// Current - The current declaration.
+      Decl *Current = nullptr;
+
+    public:
+      using value_type = Decl *;
+      using reference = const value_type &;
+      using pointer = const value_type *;
+      using iterator_category = std::forward_iterator_tag;
+      using difference_type = std::ptrdiff_t;
+
+      decl_iterator() = default;
+      explicit decl_iterator(Decl *C) : Current(C) {}
+
+      reference operator*() const { return Current; }
+
+      // This doesn't meet the iterator requirements, but it's convenient
+      value_type operator->() const { return Current; }
+
+      decl_iterator& operator++() {
+        Current = Current->getNextDeclInContext();
+        return *this;
+      }
+
+      decl_iterator operator++(int) {
+        decl_iterator tmp(*this);
+        ++(*this);
+        return tmp;
+      }
+
+      friend bool operator==(decl_iterator x, decl_iterator y) {
+        return x.Current == y.Current;
+      }
+
+      friend bool operator!=(decl_iterator x, decl_iterator y) {
+        return x.Current != y.Current;
+      }
+    };
+    */
 
   // TODO: ERICH: Put this somewhere better? Rename?
-  using DeclCollection = llvm::SmallVector<Decl*>;
+  using DeclCollection = llvm::SmallVector<Decl *>;
   mutable DeclCollection OurDecls;
 
   using decl_iterator = DeclCollection::iterator;
   using decl_range = llvm::iterator_range<decl_iterator>;
 
-
   /// decls_begin/decls_end - Iterate over the declarations stored in
   /// this context.
   decl_range decls() const { return decl_range(decls_begin(), decls_end()); }
   decl_iterator decls_begin() const;
   // TODO: ERICH: Do we need to do the 'load' decls thing here too?
-  decl_iterator decls_end() const;// { return OurDecls.end(); }
+  decl_iterator decls_end() const; // { return OurDecls.end(); }
   bool decls_empty() const;
 
   /// noload_decls_begin/end - Iterate over the declarations stored in this
@@ -2710,8 +2711,8 @@ public:
     // to be checking that D is either not last in its declaration, or is this
     // first or last decl.
     return D && llvm::find(OurDecls, D) != OurDecls.end();
-    //return D && (D->NextInContextAndBits.getPointer() || D == FirstDecl ||
-    //             D == LastDecl);
+    // return D && (D->NextInContextAndBits.getPointer() || D == FirstDecl ||
+    //              D == LastDecl);
   }
 
   void setUseQualifiedLookup(bool use = true) const {
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index c3a2779465..e999d62b45 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5095,7 +5095,7 @@ RecordDecl::field_iterator RecordDecl::field_begin() const {
     return D->field_begin();
 
   return field_iterator(decls_begin());
-  //return field_iterator(decl_iterator(FirstDecl));
+  // return field_iterator(decl_iterator(FirstDecl));
 }
 
 /// completeDefinition - Notes that the definition of this type is now
@@ -5135,10 +5135,11 @@ void RecordDecl::reorderDecls(const SmallVectorImpl<Decl *> &Decls) {
   //  We could also try harder on the assert here to make sure this is the same
   //  collection, just sorted differently.
 
-  assert(Decls.size() == OurDecls.size() && "Reordering a different set of decls?");
+  assert(Decls.size() == OurDecls.size() &&
+         "Reordering a different set of decls?");
   OurDecls.clear();
   OurDecls.insert(OurDecls.begin(), Decls.begin(), Decls.end());
-  //LastDecl->NextInContextAndBits.setPointer(nullptr);
+  // LastDecl->NextInContextAndBits.setPointer(nullptr);
   setIsRandomized(true);
 }
 
@@ -5168,13 +5169,13 @@ void RecordDecl::LoadFieldsFromExternalStorage() const {
   // FieldsAlreadyLoaded removes the filter criteria, so this is just an
   // 'insert'.
   OurDecls.insert(OurDecls.begin(), Decls.begin(), Decls.end());
-  //auto [ExternalFirst, ExternalLast] =
-  //    BuildDeclChain(Decls,
-  //                   /*FieldsAlreadyLoaded=*/false);
-  //ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
-  //FirstDecl = ExternalFirst;
-  //if (!LastDecl)
-  //  LastDecl = ExternalLast;
+  // auto [ExternalFirst, ExternalLast] =
+  //     BuildDeclChain(Decls,
+  //                    /*FieldsAlreadyLoaded=*/false);
+  // ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+  // FirstDecl = ExternalFirst;
+  // if (!LastDecl)
+  //   LastDecl = ExternalLast;
 }
 
 bool RecordDecl::mayInsertExtraPadding(bool EmitRemark) const {
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index cc67291506..2785a620e6 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1523,26 +1523,25 @@ void DeclContext::collectAllContexts(SmallVectorImpl<DeclContext *> &Contexts) {
     Contexts.push_back(this);
 }
 
-//std::pair<Decl *, Decl *>
-//DeclContext::BuildDeclChain(ArrayRef<Decl *> Decls,
-//                            bool FieldsAlreadyLoaded) {
-//  // Build up a chain of declarations via the Decl::NextInContextAndBits field.
-//  Decl *FirstNewDecl = nullptr;
-//  Decl *PrevDecl = nullptr;
-//  for (auto *D : Decls) {
-//    if (FieldsAlreadyLoaded && isa<FieldDecl>(D))
-//      continue;
+// std::pair<Decl *, Decl *>
+// DeclContext::BuildDeclChain(ArrayRef<Decl *> Decls,
+//                             bool FieldsAlreadyLoaded) {
+//   // Build up a chain of declarations via the Decl::NextInContextAndBits
+//   field. Decl *FirstNewDecl = nullptr; Decl *PrevDecl = nullptr; for (auto *D
+//   : Decls) {
+//     if (FieldsAlreadyLoaded && isa<FieldDecl>(D))
+//       continue;
 //
-//    if (PrevDecl)
-//      PrevDecl->NextInContextAndBits.setPointer(D);
-//    else
-//      FirstNewDecl = D;
+//     if (PrevDecl)
+//       PrevDecl->NextInContextAndBits.setPointer(D);
+//     else
+//       FirstNewDecl = D;
 //
-//    PrevDecl = D;
-//  }
+//     PrevDecl = D;
+//   }
 //
-//  return std::make_pair(FirstNewDecl, PrevDecl);
-//}
+//   return std::make_pair(FirstNewDecl, PrevDecl);
+// }
 
 /// We have just acquired external visible storage, and we already have
 /// built a lookup map. For every name in the map, pull in the new names from
@@ -1582,12 +1581,12 @@ DeclContext::LoadLexicalDeclsFromExternalStorage() const {
 
   // Splice the newly-read declarations into the beginning of the list
   // of declarations.
-  //Decl *ExternalFirst, *ExternalLast;
-  //std::tie(ExternalFirst, ExternalLast) =
+  // Decl *ExternalFirst, *ExternalLast;
+  // std::tie(ExternalFirst, ExternalLast) =
   //    BuildDeclChain(Decls, FieldsAlreadyLoaded);
-  //ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
-  //FirstDecl = ExternalFirst;
-  //if (!LastDecl)
+  // ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+  // FirstDecl = ExternalFirst;
+  // if (!LastDecl)
   //  LastDecl = ExternalLast;
 
   // TODO: ERICH: It isn't clear why these decls are going at the beginning,
@@ -1635,7 +1634,7 @@ DeclContext::decl_iterator DeclContext::decls_begin() const {
   if (hasExternalLexicalStorage())
     LoadLexicalDeclsFromExternalStorage();
   return OurDecls.begin();
-  //return decl_iterator(FirstDecl);
+  // return decl_iterator(FirstDecl);
 }
 
 // TODO: ERICH: we need this now, since we don't know the order of the
@@ -1655,7 +1654,7 @@ bool DeclContext::decls_empty() const {
     LoadLexicalDeclsFromExternalStorage();
 
   return OurDecls.empty();
-//  return !FirstDecl;
+  //  return !FirstDecl;
 }
 
 bool DeclContext::containsDecl(Decl *D) const {
@@ -1727,24 +1726,25 @@ void DeclContext::removeDecl(Decl *D) {
   OurDecls.erase(Itr);
 
   //// Remove D from the decl chain.  This is O(n) but hopefully rare.{
-  //if (D == FirstDecl) {
-  //  if (D == LastDecl)
-  //    FirstDecl = LastDecl = nullptr;
-  //  else
-  //    FirstDecl = D->NextInContextAndBits.getPointer();
-  //} else {
-  //  for (Decl *I = FirstDecl; true; I = I->NextInContextAndBits.getPointer()) {
-  //    assert(I && "decl not found in linked list");
-  //    if (I->NextInContextAndBits.getPointer() == D) {
-  //      I->NextInContextAndBits.setPointer(D->NextInContextAndBits.getPointer());
-  //      if (D == LastDecl) LastDecl = I;
-  //      break;
-  //    }
-  //  }
-  //}
+  // if (D == FirstDecl) {
+  //   if (D == LastDecl)
+  //     FirstDecl = LastDecl = nullptr;
+  //   else
+  //     FirstDecl = D->NextInContextAndBits.getPointer();
+  // } else {
+  //   for (Decl *I = FirstDecl; true; I = I->NextInContextAndBits.getPointer())
+  //   {
+  //     assert(I && "decl not found in linked list");
+  //     if (I->NextInContextAndBits.getPointer() == D) {
+  //       I->NextInContextAndBits.setPointer(D->NextInContextAndBits.getPointer());
+  //       if (D == LastDecl) LastDecl = I;
+  //       break;
+  //     }
+  //   }
+  // }
 
   //// Mark that D is no longer in the decl chain.
-  //D->NextInContextAndBits.setPointer(nullptr);
+  // D->NextInContextAndBits.setPointer(nullptr);
 
   // Remove D from the lookup table if necessary.
   if (isa<NamedDecl>(D)) {
@@ -1785,16 +1785,16 @@ void DeclContext::addHiddenDecl(Decl *D) {
   assert(llvm::find(OurDecls, D) == OurDecls.end() &&
          "decl already inserted into this DeclContext");
   // TODO: this isn't really possible now,
-  //assert(!D->getNextDeclInContext() && D != LastDecl &&
+  // assert(!D->getNextDeclInContext() && D != LastDecl &&
   //       "Decl already inserted into a DeclContext");
 
   OurDecls.push_back(D);
-//  if (FirstDecl) {
-//    LastDecl->NextInContextAndBits.setPointer(D);
-//    LastDecl = D;
-//  } else {
-//    FirstDecl = LastDecl = D;
-//  }
+  //  if (FirstDecl) {
+  //    LastDecl->NextInContextAndBits.setPointer(D);
+  //    LastDecl = D;
+  //  } else {
+  //    FirstDecl = LastDecl = D;
+  //  }
 
   // Notify a C++ record declaration that we've added a member, so it can
   // update its class-specific state.
@@ -2024,7 +2024,7 @@ void DeclContext::localUncachedLookup(DeclarationName Name,
   // matches.
   // FIXME: If we have lazy external declarations, this will not find them!
   // FIXME: Should we CollectAllContexts and walk them all here?
-  //for (Decl *D = FirstDecl; D; D = D->getNextDeclInContext()) {
+  // for (Decl *D = FirstDecl; D; D = D->getNextDeclInContext()) {
   //  if (auto *ND = dyn_cast<NamedDecl>(D))
   //    if (ND->getDeclName() == Name)
   //      Results.push_back(ND);

``````````

</details>


https://github.com/llvm/llvm-project/pull/113983


More information about the cfe-commits mailing list