https://github.com/daniel-grumberg updated https://github.com/llvm/llvm-project/pull/104600
>From 8e3909ecb1bfe6aec6344cd89cbe1798d6cde7da Mon Sep 17 00:00:00 2001 From: Daniel Grumberg <dgrumb...@apple.com> Date: Thu, 15 Aug 2024 17:42:02 +0100 Subject: [PATCH 1/4] [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 6b373a5d8542f0f5b6dc23d322aab7d520377c22 Mon Sep 17 00:00:00 2001 From: Daniel Grumberg <dgrumb...@apple.com> Date: Thu, 15 Aug 2024 17:43:26 +0100 Subject: [PATCH 2/4] [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 1f8029cbd39ad2..1bce9c59b19791 100644 --- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp +++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp @@ -673,14 +673,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 5769b2af891442c5bd78e980f69b5793649b97b4 Mon Sep 17 00:00:00 2001 From: Daniel Grumberg <dgrumb...@apple.com> Date: Thu, 15 Aug 2024 17:46:30 +0100 Subject: [PATCH 3/4] [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@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 >From ee2b8334c25d6f7073b3df5c8f56adde350755c5 Mon Sep 17 00:00:00 2001 From: Daniel Grumberg <dgrumb...@apple.com> Date: Fri, 16 Aug 2024 16:04:23 +0100 Subject: [PATCH 4/4] Apply git clang-format --- clang/include/clang/ExtractAPI/API.h | 3 ++- .../include/clang/ExtractAPI/ExtractAPIVisitor.h | 3 ++- clang/lib/ExtractAPI/API.cpp | 16 ++++++---------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/ExtractAPI/API.h b/clang/include/clang/ExtractAPI/API.h index 3b36dfe0325b9b..188e35b72117b5 100644 --- a/clang/include/clang/ExtractAPI/API.h +++ b/clang/include/clang/ExtractAPI/API.h @@ -1421,7 +1421,8 @@ class APISet { createRecord(StringRef USR, StringRef Name, CtorArgsContTy &&...CtorArgs); auto getTopLevelRecords() const { - return llvm::iterator_range<decltype(TopLevelRecords)::iterator>(TopLevelRecords); + return llvm::iterator_range<decltype(TopLevelRecords)::iterator>( + TopLevelRecords); } void removeRecord(StringRef USR); diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h index a5e4dbbe04a165..8d9ac062034511 100644 --- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h +++ b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h @@ -606,7 +606,8 @@ bool ExtractAPIVisitorBase<Derived>::VisitRecordDecl(const RecordDecl *Decl) { } template <typename Derived> -bool ExtractAPIVisitorBase<Derived>::TraverseCXXRecordDecl(CXXRecordDecl *Decl) { +bool ExtractAPIVisitorBase<Derived>::TraverseCXXRecordDecl( + CXXRecordDecl *Decl) { bool Ret = Base::TraverseCXXRecordDecl(Decl); if (!isEmbeddedInVarDeclarator(*Decl) && Decl->isAnonymousStructOrUnion()) { diff --git a/clang/lib/ExtractAPI/API.cpp b/clang/lib/ExtractAPI/API.cpp index 48d8bb7f600630..9dbc023885c37f 100644 --- a/clang/lib/ExtractAPI/API.cpp +++ b/clang/lib/ExtractAPI/API.cpp @@ -70,7 +70,8 @@ void RecordContext::stealRecordChain(RecordContext &Other) { Last = Other.Last; - for (auto *StolenRecord = Other.First; StolenRecord != nullptr; StolenRecord = StolenRecord->getNextInContext()) + 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. @@ -135,25 +136,22 @@ 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 &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)) + if (auto *RecordAsCtx = llvm::dyn_cast<RecordContext>(Record)) ParentCtx->stealRecordChain(*RecordAsCtx); } else { TopLevelRecords.erase(Record); - if (auto *RecordAsCtx= - llvm::dyn_cast<RecordContext>(Record)) { + if (auto *RecordAsCtx = llvm::dyn_cast<RecordContext>(Record)) { for (const auto *Child = RecordAsCtx->First; Child != nullptr; Child = Child->getNextInContext()) TopLevelRecords.insert(Child); @@ -163,9 +161,7 @@ void APISet::removeRecord(StringRef USR) { } } -void APISet::removeRecord(APIRecord *Record) { - removeRecord(Record->USR); -} +void APISet::removeRecord(APIRecord *Record) { removeRecord(Record->USR); } APIRecord::~APIRecord() {} TagRecord::~TagRecord() {} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits