Skip to content

Reenable anon structs #104922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions clang/include/clang/ExtractAPI/API.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -328,6 +320,8 @@ class RecordContext {
/// chain.
void stealRecordChain(RecordContext &Other);

void removeFromRecordChain(APIRecord *Record);

APIRecord::RecordKind getKind() const { return Kind; }

struct record_iterator {
Expand Down Expand Up @@ -1426,10 +1420,15 @@ 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) {}
Expand All @@ -1456,7 +1455,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;
Expand All @@ -1482,7 +1481,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());
}
Expand Down
37 changes: 35 additions & 2 deletions clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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) {}
Expand Down Expand Up @@ -81,8 +83,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);
Expand Down Expand Up @@ -240,7 +244,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,
Expand All @@ -252,8 +256,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);
}
}
}
};
Expand Down Expand Up @@ -548,6 +554,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))
Expand Down Expand Up @@ -588,6 +607,20 @@ 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) {
Expand Down
53 changes: 51 additions & 2 deletions clang/lib/ExtractAPI/API.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand All @@ -68,6 +70,10 @@ 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;
Expand All @@ -85,6 +91,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;
Expand Down Expand Up @@ -114,6 +136,33 @@ 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() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("_"))
Expand Down
12 changes: 12 additions & 0 deletions clang/test/ExtractAPI/anonymous_record_no_typedef.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading