Revert "[clang][ExtractAPI] Stop dropping fields of nested anonymous record types when they aren't attached to variable declaration (#104600)"

This reverts commit c60da1a.

Author: daniel-grumberg

clang/include/clang/ExtractAPI/API.h

@@ -19,13 +19,21 @@
 #define LLVM_CLANG_EXTRACTAPI_API_H
 
 #include "clang/AST/Availability.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
-#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Triple.h"
 #include <cstddef>
 #include <iterator>
@@ -320,8 +328,6 @@ class RecordContext {
   /// chain.
   void stealRecordChain(RecordContext &Other);
 
-  void removeFromRecordChain(APIRecord *Record);
-
   APIRecord::RecordKind getKind() const { return Kind; }
 
   struct record_iterator {
@@ -1420,15 +1426,10 @@ class APISet {
   typename std::enable_if_t<std::is_base_of_v<APIRecord, RecordTy>, RecordTy> *
   createRecord(StringRef USR, StringRef Name, CtorArgsContTy &&...CtorArgs);
 
-  auto getTopLevelRecords() const {
-    return llvm::iterator_range<decltype(TopLevelRecords)::iterator>(
-        TopLevelRecords);
+  ArrayRef<const APIRecord *> getTopLevelRecords() const {
+    return TopLevelRecords;
   }
 
-  void removeRecord(StringRef USR);
-
-  void removeRecord(APIRecord *Record);
-
   APISet(const llvm::Triple &Target, Language Lang,
          const std::string &ProductName)
       : Target(Target), Lang(Lang), ProductName(ProductName) {}
@@ -1455,7 +1456,7 @@ class APISet {
   // lives in the BumpPtrAllocator.
   using APIRecordStoredPtr = std::unique_ptr<APIRecord, APIRecordDeleter>;
   llvm::DenseMap<StringRef, APIRecordStoredPtr> USRBasedLookupTable;
-  llvm::SmallPtrSet<const APIRecord *, 32> TopLevelRecords;
+  std::vector<const APIRecord *> TopLevelRecords;
 
 public:
   const std::string ProductName;
@@ -1481,7 +1482,7 @@ APISet::createRecord(StringRef USR, StringRef Name,
             dyn_cast_if_present<RecordContext>(Record->Parent.Record))
       ParentContext->addToRecordChain(Record);
     else
-      TopLevelRecords.insert(Record);
+      TopLevelRecords.push_back(Record);
   } else {
     Record = dyn_cast<RecordTy>(Result.first->second.get());
   }

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

@@ -23,11 +23,13 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Module.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/ExtractAPI/API.h"
 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "clang/ExtractAPI/TypedefUnderlyingTypeResolver.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include <type_traits>
@@ -38,8 +40,6 @@ namespace impl {
 
 template <typename Derived>
 class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
-  using Base = RecursiveASTVisitor<Derived>;
-
 protected:
   ExtractAPIVisitorBase(ASTContext &Context, APISet &API)
       : Context(Context), API(API) {}
@@ -81,10 +81,8 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
 
   bool VisitNamespaceDecl(const NamespaceDecl *Decl);
 
-  bool TraverseRecordDecl(RecordDecl *Decl);
   bool VisitRecordDecl(const RecordDecl *Decl);
 
-  bool TraverseCXXRecordDecl(CXXRecordDecl *Decl);
   bool VisitCXXRecordDecl(const CXXRecordDecl *Decl);
 
   bool VisitCXXMethodDecl(const CXXMethodDecl *Decl);
@@ -242,7 +240,7 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
 
   bool isEmbeddedInVarDeclarator(const TagDecl &D) {
     return D.getName().empty() && getTypedefName(&D).empty() &&
-           D.isEmbeddedInDeclarator() && !D.isFreeStanding();
+           D.isEmbeddedInDeclarator();
   }
 
   void maybeMergeWithAnonymousTag(const DeclaratorDecl &D,
@@ -254,10 +252,8 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
     clang::index::generateUSRForDecl(Tag, TagUSR);
     if (auto *Record = llvm::dyn_cast_if_present<TagRecord>(
             API.findRecordForUSR(TagUSR))) {
-      if (Record->IsEmbeddedInVarDeclarator) {
+      if (Record->IsEmbeddedInVarDeclarator)
         NewRecordContext->stealRecordChain(*Record);
-        API.removeRecord(Record);
-      }
     }
   }
 };
@@ -552,19 +548,6 @@ bool ExtractAPIVisitorBase<Derived>::VisitNamespaceDecl(
   return true;
 }
 
-template <typename Derived>
-bool ExtractAPIVisitorBase<Derived>::TraverseRecordDecl(RecordDecl *Decl) {
-  bool Ret = Base::TraverseRecordDecl(Decl);
-
-  if (!isEmbeddedInVarDeclarator(*Decl) && Decl->isAnonymousStructOrUnion()) {
-    SmallString<128> USR;
-    index::generateUSRForDecl(Decl, USR);
-    API.removeRecord(USR);
-  }
-
-  return Ret;
-}
-
 template <typename Derived>
 bool ExtractAPIVisitorBase<Derived>::VisitRecordDecl(const RecordDecl *Decl) {
   if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
@@ -605,20 +588,6 @@ bool ExtractAPIVisitorBase<Derived>::VisitRecordDecl(const RecordDecl *Decl) {
   return true;
 }
 
-template <typename Derived>
-bool ExtractAPIVisitorBase<Derived>::TraverseCXXRecordDecl(
-    CXXRecordDecl *Decl) {
-  bool Ret = Base::TraverseCXXRecordDecl(Decl);
-
-  if (!isEmbeddedInVarDeclarator(*Decl) && Decl->isAnonymousStructOrUnion()) {
-    SmallString<128> USR;
-    index::generateUSRForDecl(Decl, USR);
-    API.removeRecord(USR);
-  }
-
-  return Ret;
-}
-
 template <typename Derived>
 bool ExtractAPIVisitorBase<Derived>::VisitCXXRecordDecl(
     const CXXRecordDecl *Decl) {

clang/lib/ExtractAPI/API.cpp

@@ -13,6 +13,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/ExtractAPI/API.h"
+#include "clang/AST/RawCommentList.h"
+#include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <memory>
@@ -58,10 +60,6 @@ bool RecordContext::IsWellFormed() const {
 
 void RecordContext::stealRecordChain(RecordContext &Other) {
   assert(IsWellFormed());
-  // Other's record chain is empty, nothing to do
-  if (Other.First == nullptr && Other.Last == nullptr)
-    return;
-
   // If we don't have an empty chain append Other's chain into ours.
   if (First)
     Last->NextInContext = Other.First;
@@ -70,10 +68,6 @@ void RecordContext::stealRecordChain(RecordContext &Other) {
 
   Last = Other.Last;
 
-  for (auto *StolenRecord = Other.First; StolenRecord != nullptr;
-       StolenRecord = StolenRecord->getNextInContext())
-    StolenRecord->Parent = SymbolReference(cast<APIRecord>(this));
-
   // Delete Other's chain to ensure we don't accidentally traverse it.
   Other.First = nullptr;
   Other.Last = nullptr;
@@ -91,22 +85,6 @@ void RecordContext::addToRecordChain(APIRecord *Record) const {
   Last = Record;
 }
 
-void RecordContext::removeFromRecordChain(APIRecord *Record) {
-  APIRecord *Prev = nullptr;
-  for (APIRecord *Curr = First; Curr != Record; Curr = Curr->NextInContext)
-    Prev = Curr;
-
-  if (Prev)
-    Prev->NextInContext = Record->NextInContext;
-  else
-    First = Record->NextInContext;
-
-  if (Last == Record)
-    Last = Prev;
-
-  Record->NextInContext = nullptr;
-}
-
 APIRecord *APISet::findRecordForUSR(StringRef USR) const {
   if (USR.empty())
     return nullptr;
@@ -136,33 +114,6 @@ SymbolReference APISet::createSymbolReference(StringRef Name, StringRef USR,
   return SymbolReference(copyString(Name), copyString(USR), copyString(Source));
 }
 
-void APISet::removeRecord(StringRef USR) {
-  auto Result = USRBasedLookupTable.find(USR);
-  if (Result != USRBasedLookupTable.end()) {
-    auto *Record = Result->getSecond().get();
-    auto &ParentReference = Record->Parent;
-    auto *ParentRecord = const_cast<APIRecord *>(ParentReference.Record);
-    if (!ParentRecord)
-      ParentRecord = findRecordForUSR(ParentReference.USR);
-
-    if (auto *ParentCtx = llvm::cast_if_present<RecordContext>(ParentRecord)) {
-      ParentCtx->removeFromRecordChain(Record);
-      if (auto *RecordAsCtx = llvm::dyn_cast<RecordContext>(Record))
-        ParentCtx->stealRecordChain(*RecordAsCtx);
-    } else {
-      TopLevelRecords.erase(Record);
-      if (auto *RecordAsCtx = llvm::dyn_cast<RecordContext>(Record)) {
-        for (const auto *Child = RecordAsCtx->First; Child != nullptr;
-             Child = Child->getNextInContext())
-          TopLevelRecords.insert(Child);
-      }
-    }
-    USRBasedLookupTable.erase(Result);
-  }
-}
-
-void APISet::removeRecord(APIRecord *Record) { removeRecord(Record->USR); }
-
 APIRecord::~APIRecord() {}
 TagRecord::~TagRecord() {}
 RecordRecord::~RecordRecord() {}

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

@@ -673,6 +673,14 @@ bool SymbolGraphSerializer::shouldSkip(const APIRecord *Record) const {
   if (Record->Availability.isUnconditionallyUnavailable())
     return true;
 
+  // Filter out symbols without a name as we can generate correct symbol graphs
+  // for them. In practice these are anonymous record types that aren't attached
+  // to a declaration.
+  if (auto *Tag = dyn_cast<TagRecord>(Record)) {
+    if (Tag->IsEmbeddedInVarDeclarator)
+      return true;
+  }
+
   // Filter out symbols prefixed with an underscored as they are understood to
   // be symbols clients should not use.
   if (Record->Name.starts_with("_"))

clang/test/ExtractAPI/anonymous_record_no_typedef.c

@@ -163,16 +163,4 @@ enum {
 // GLOBALOTHERCASE-NEXT:   "GlobalOtherCase"
 // GLOBALOTHERCASE-NEXT: ]
 
-// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix VEC
-union Vector {
-  struct {
-    float X;
-    float Y;
-  };
-  float Data[2];
-};
-// VEC-DAG: "!testRelLabel": "memberOf $ c:@U@Vector@FI@Data $ c:@U@Vector"
-// VEC-DAG: "!testRelLabel": "memberOf $ c:@U@Vector@Sa@FI@X $ c:@U@Vector"
-// VEC-DAG: "!testRelLabel": "memberOf $ c:@U@Vector@Sa@FI@Y $ c:@U@Vector"
-
 // expected-no-diagnostics