Author: Daniel Grumberg Date: 2024-08-19T16:06:43+01:00 New Revision: b18b4547f1bfaf6da37b29440a96176e807c2e6c
URL: https://github.com/llvm/llvm-project/commit/b18b4547f1bfaf6da37b29440a96176e807c2e6c DIFF: https://github.com/llvm/llvm-project/commit/b18b4547f1bfaf6da37b29440a96176e807c2e6c.diff LOG: Revert "[clang][ExtractAPI] Stop dropping fields of nested anonymous record types when they aren't attached to variable declaration (#104600)" This reverts commit c60da1a271a6bb271e7703b2f7c71fbece67ab78. Added: Modified: clang/include/clang/ExtractAPI/API.h clang/include/clang/ExtractAPI/ExtractAPIVisitor.h clang/lib/ExtractAPI/API.cpp clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp clang/test/ExtractAPI/anonymous_record_no_typedef.c Removed: ################################################################################ diff --git a/clang/include/clang/ExtractAPI/API.h b/clang/include/clang/ExtractAPI/API.h index 188e35b72117b5..bf291074fd0614 100644 --- a/clang/include/clang/ExtractAPI/API.h +++ b/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()); } diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h index 8d9ac062034511..1b27027621666a 100644 --- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h +++ b/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) { diff --git a/clang/lib/ExtractAPI/API.cpp b/clang/lib/ExtractAPI/API.cpp index 9dbc023885c37f..ab1108f663deac 100644 --- a/clang/lib/ExtractAPI/API.cpp +++ b/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() {} diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp index 1bce9c59b19791..1f8029cbd39ad2 100644 --- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp +++ b/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("_")) diff --git a/clang/test/ExtractAPI/anonymous_record_no_typedef.c b/clang/test/ExtractAPI/anonymous_record_no_typedef.c index 064c223ad56e73..789316ca8930b8 100644 --- a/clang/test/ExtractAPI/anonymous_record_no_typedef.c +++ b/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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
