Skip to content

[clang][ExtractAPI] combine typedef records if the underlying type's name is underscored #125964

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.

Sign up for GitHub

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
Feb 7, 2025
Merged

[clang][ExtractAPI] combine typedef records if the underlying type's name is underscored #125964

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75a724e
combine typedef records if the underlying type's name is underscored
QuietMisdreavus Feb 5, 2025
25a71ea
review: use ltrim instead of drop_while
QuietMisdreavus Feb 6, 2025
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
26 changes: 25 additions & 1 deletion clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1146,11 +1146,30 @@ bool ExtractAPIVisitorBase<Derived>::VisitTypedefNameDecl(

StringRef Name = Decl->getName();

auto nameMatches = [&Name](TagDecl *TagDecl) {
StringRef TagName = TagDecl->getName();

if (TagName == Name)
return true;

// Also check whether the tag decl's name is the same as the typedef name
// with prefixed underscores
if (TagName.starts_with('_')) {
Copy link
Contributor

@PortalPete PortalPete Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth catching a typedef of a typedef that also begins with an underscore?

  • This method could catch daisy-chain types by putting these lines in a loop.
  • To avoid an ∞ loop (albeit, rather unlikely), it could have an arbitrary iteration limit of something reasonable, say 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already limiting itself to underlying types which (1) are "tag decls", i.e. structs/classes/enums/unions, and (2) whose definitions are "embedded in a declarator", which AFAIK means that it's not going to catch typedefs of typedefs to begin with.

Now, I recently implemented logic in the Swift compiler to catch the same situation i mentioned in the PR description, but in that PR i took a more general approach and folded together "public type aliases of types that are being hidden (e.g. underscored)". However, the Swift symbol graph generator didn't have its own notion of "folding decls together" yet, since in the average case the Swift Clang Importer already handles that, whereas here in Clang we were already handling typedef struct unification. (I also didn't go recursive in the Swift PR, but the nondeterministic ordering of AST walking could make that even more complicated! 😵‍💫)

I'm inclined to hold off on going recursive for now, and come back later if we need to handle typedef _HiddenTypedef PublicTypedef in this kind of way. I could see this becoming even more complicated in this kind of situation, to avoid revealing too many implementation details in an ostensibly public symbol graph rendering.

StringRef StrippedName =
TagName.drop_while([](char c) { return c == '_'; });

if (StrippedName == Name)
return true;
}

return false;
};

// If the underlying type was defined as part of the typedef modify it's
// fragments directly and pretend the typedef doesn't exist.
if (auto *TagDecl = Decl->getUnderlyingType()->getAsTagDecl()) {
if (TagDecl->isEmbeddedInDeclarator() && TagDecl->isCompleteDefinition() &&
Decl->getName() == TagDecl->getName()) {
nameMatches(TagDecl)) {
SmallString<128> TagUSR;
index::generateUSRForDecl(TagDecl, TagUSR);
if (auto *Record = API.findRecordForUSR(TagUSR)) {
Expand All @@ -1164,6 +1183,11 @@ bool ExtractAPIVisitorBase<Derived>::VisitTypedefNameDecl(
.append(Name, DeclarationFragments::FragmentKind::Identifier)
.appendSemicolon();

// Replace the name and subheading in case it's underscored so we can
// use the non-underscored version
Record->Name = Name;
Record->SubHeading = DeclarationFragmentsBuilder::getSubHeading(Decl);

return true;
}
}
Expand Down
69 changes: 69 additions & 0 deletions clang/test/ExtractAPI/typedef_underscore.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -extract-api --pretty-sgf --emit-sgf-symbol-labels-for-testing \
// RUN: --product-name=TypedefChain -triple arm64-apple-macosx -x c-header %s -o %t/typedefchain-c.symbols.json -verify
// RUN: %clang_cc1 -extract-api --pretty-sgf --emit-sgf-symbol-labels-for-testing \
// RUN: --product-name=TypedefChain -triple arm64-apple-macosx -x c++-header %s -o %t/typedefchain-cxx.symbols.json -verify

// RUN: FileCheck %s --input-file %t/typedefchain-c.symbols.json --check-prefix MYSTRUCT
// RUN: FileCheck %s --input-file %t/typedefchain-cxx.symbols.json --check-prefix MYSTRUCT
typedef struct _MyStruct { } MyStruct;

// MYSTRUCT-LABEL: "!testLabel": "c:@S@_MyStruct"
// MYSTRUCT: "accessLevel": "public",
// MYSTRUCT: "declarationFragments": [
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "keyword",
// MYSTRUCT-NEXT: "spelling": "typedef"
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "text",
// MYSTRUCT-NEXT: "spelling": " "
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "keyword",
// MYSTRUCT-NEXT: "spelling": "struct"
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "text",
// MYSTRUCT-NEXT: "spelling": " "
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "identifier",
// MYSTRUCT-NEXT: "spelling": "_MyStruct"
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "text",
// MYSTRUCT-NEXT: "spelling": " { ... } "
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "identifier",
// MYSTRUCT-NEXT: "spelling": "MyStruct"
// MYSTRUCT-NEXT: },
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "text",
// MYSTRUCT-NEXT: "spelling": ";"
// MYSTRUCT-NEXT: }
// MYSTRUCT-NEXT: ],
// MYSTRUCT: "kind": {
// MYSTRUCT-NEXT: "displayName": "Structure",
// MYSTRUCT-NEXT: "identifier": "c{{(\+\+)?}}.struct"
// MYSTRUCT: "names": {
// MYSTRUCT-NEXT: "navigator": [
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "identifier",
// MYSTRUCT-NEXT: "spelling": "MyStruct"
// MYSTRUCT-NEXT: }
// MYSTRUCT-NEXT: ],
// MYSTRUCT-NEXT: "subHeading": [
// MYSTRUCT-NEXT: {
// MYSTRUCT-NEXT: "kind": "identifier",
// MYSTRUCT-NEXT: "spelling": "MyStruct"
// MYSTRUCT-NEXT: }
// MYSTRUCT-NEXT: ],
// MYSTRUCT-NEXT: "title": "MyStruct"
// MYSTRUCT-NEXT: },
// MYSTRUCT: "pathComponents": [
// MYSTRUCT-NEXT: "MyStruct"
// MYSTRUCT-NEXT: ]

// expected-no-diagnostics