[clang] [clang][ExtractAPI] Stop dropping fields of nested anonymous record types when they aren't attached to variable declaration (PR #104600)
Daniel Grumberg via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 16 07:56:05 PDT 2024
https://github.com/daniel-grumberg created https://github.com/llvm/llvm-project/pull/104600
- Introduce primitives for removing records from `APISet` and managing the record chain of `RecordContext`
- Detect nested anonymous record types and remove them from the `APISet` after they have been fully traversed and transfer ownership of child records to the parent context (if any)
>From 61b8014d418867e0108bf4513227d9a8fdad63f8 Mon Sep 17 00:00:00 2001
From: Daniel Grumberg <dgrumberg at apple.com>
Date: Thu, 15 Aug 2024 17:42:02 +0100
Subject: [PATCH 1/3] [clang][ExtractAPI] Implement Record removal from APISet
rdar://128092236
---
clang/include/clang/ExtractAPI/API.h | 24 ++++++------
clang/lib/ExtractAPI/API.cpp | 57 +++++++++++++++++++++++++++-
2 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/ExtractAPI/API.h b/clang/include/clang/ExtractAPI/API.h
index bf291074fd0614..3b36dfe0325b9b 100644
--- a/clang/include/clang/ExtractAPI/API.h
+++ b/clang/include/clang/ExtractAPI/API.h
@@ -19,21 +19,13 @@
#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/ArrayRef.h"
-#include "llvm/ADT/MapVector.h"
-#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/SmallPtrSet.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>
@@ -328,6 +320,8 @@ class RecordContext {
/// chain.
void stealRecordChain(RecordContext &Other);
+ void removeFromRecordChain(APIRecord *Record);
+
APIRecord::RecordKind getKind() const { return Kind; }
struct record_iterator {
@@ -1426,10 +1420,14 @@ class APISet {
typename std::enable_if_t<std::is_base_of_v<APIRecord, RecordTy>, RecordTy> *
createRecord(StringRef USR, StringRef Name, CtorArgsContTy &&...CtorArgs);
- ArrayRef<const APIRecord *> getTopLevelRecords() const {
- return TopLevelRecords;
+ auto getTopLevelRecords() const {
+ return llvm::iterator_range<decltype(TopLevelRecords)::iterator>(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) {}
@@ -1456,7 +1454,7 @@ class APISet {
// lives in the BumpPtrAllocator.
using APIRecordStoredPtr = std::unique_ptr<APIRecord, APIRecordDeleter>;
llvm::DenseMap<StringRef, APIRecordStoredPtr> USRBasedLookupTable;
- std::vector<const APIRecord *> TopLevelRecords;
+ llvm::SmallPtrSet<const APIRecord *, 32> TopLevelRecords;
public:
const std::string ProductName;
@@ -1482,7 +1480,7 @@ APISet::createRecord(StringRef USR, StringRef Name,
dyn_cast_if_present<RecordContext>(Record->Parent.Record))
ParentContext->addToRecordChain(Record);
else
- TopLevelRecords.push_back(Record);
+ TopLevelRecords.insert(Record);
} else {
Record = dyn_cast<RecordTy>(Result.first->second.get());
}
diff --git a/clang/lib/ExtractAPI/API.cpp b/clang/lib/ExtractAPI/API.cpp
index ab1108f663deac..48d8bb7f600630 100644
--- a/clang/lib/ExtractAPI/API.cpp
+++ b/clang/lib/ExtractAPI/API.cpp
@@ -13,8 +13,6 @@
//===----------------------------------------------------------------------===//
#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>
@@ -60,6 +58,10 @@ 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;
@@ -68,6 +70,9 @@ 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;
@@ -85,6 +90,22 @@ 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;
@@ -114,6 +135,38 @@ 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() {}
>From d667d6408116670c605d9c133eb771abf92fd558 Mon Sep 17 00:00:00 2001
From: Daniel Grumberg <dgrumberg at apple.com>
Date: Thu, 15 Aug 2024 17:43:26 +0100
Subject: [PATCH 2/3] [clang][ExtractAPI] Remove stale anonymous records
Remove stale anonymous records representing the type of a VarDecl after
we migrate the children to the record representing the variable.
rdar://128092236
---
clang/include/clang/ExtractAPI/ExtractAPIVisitor.h | 8 ++++----
.../ExtractAPI/Serialization/SymbolGraphSerializer.cpp | 8 --------
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
index 1b27027621666a..1e3df42a8cb7a1 100644
--- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -23,13 +23,11 @@
#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>
@@ -240,7 +238,7 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
bool isEmbeddedInVarDeclarator(const TagDecl &D) {
return D.getName().empty() && getTypedefName(&D).empty() &&
- D.isEmbeddedInDeclarator();
+ D.isEmbeddedInDeclarator() && !D.isFreeStanding();
}
void maybeMergeWithAnonymousTag(const DeclaratorDecl &D,
@@ -252,8 +250,10 @@ 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);
+ }
}
}
};
diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
index 6e56ee5b573f66..28dcea1c691675 100644
--- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -666,14 +666,6 @@ 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("_"))
>From e4a4ff0649bac9710b8e77416b77526f16d4762b Mon Sep 17 00:00:00 2001
From: Daniel Grumberg <dgrumberg at apple.com>
Date: Thu, 15 Aug 2024 17:46:30 +0100
Subject: [PATCH 3/3] [clang][ExtractAPI] Flatten children of nested anonymous
records into the parent scope
When an anonymous record is nested within another record and is not
attached to a variable's type, we migrate the children to the parent
record.
rdar://128092236
---
.../clang/ExtractAPI/ExtractAPIVisitor.h | 30 +++++++++++++++++++
.../ExtractAPI/anonymous_record_no_typedef.c | 12 ++++++++
2 files changed, 42 insertions(+)
diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
index 1e3df42a8cb7a1..a5e4dbbe04a165 100644
--- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -38,6 +38,8 @@ namespace impl {
template <typename Derived>
class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
+ using Base = RecursiveASTVisitor<Derived>;
+
protected:
ExtractAPIVisitorBase(ASTContext &Context, APISet &API)
: Context(Context), API(API) {}
@@ -79,8 +81,10 @@ 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);
@@ -548,6 +552,19 @@ 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))
@@ -588,6 +605,19 @@ 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) {
diff --git a/clang/test/ExtractAPI/anonymous_record_no_typedef.c b/clang/test/ExtractAPI/anonymous_record_no_typedef.c
index 789316ca8930b8..064c223ad56e73 100644
--- a/clang/test/ExtractAPI/anonymous_record_no_typedef.c
+++ b/clang/test/ExtractAPI/anonymous_record_no_typedef.c
@@ -163,4 +163,16 @@ 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 at Vector@FI at Data $ c:@U at Vector"
+// VEC-DAG: "!testRelLabel": "memberOf $ c:@U at Vector@Sa at FI@X $ c:@U at Vector"
+// VEC-DAG: "!testRelLabel": "memberOf $ c:@U at Vector@Sa at FI@Y $ c:@U at Vector"
+
// expected-no-diagnostics
More information about the cfe-commits
mailing list