This is an archive of the discontinued LLVM Phabricator instance.

[clang][extract-api] Add support for typedefs
ClosedPublic

Authored by dang on Apr 4 2022, 3:54 AM.

Details

Summary

Typedef records consist of the symbol associated with the underlying
TypedefDecl and a SymbolReference to the underlying type. Additionally
typedefs for anonymous TagTypes use the typedef'd name as the symbol
name in their respective records and USRs. As a result the declaration
fragments for the anonymous TagType are those for the associated
typedef. This means that when the user is defining a typedef to a
typedef to a anonymous type, we use a reference the anonymous TagType
itself and do not emit the typedef to the anonymous type in the
generated symbol graph, including in the type destination of further
typedef symbol records.

Diff Detail

Event Timeline

dang created this revision.Apr 4 2022, 3:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2022, 3:54 AM
Herald added a subscriber: mgorny. · View Herald Transcript
dang requested review of this revision.Apr 4 2022, 3:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2022, 3:54 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
dang retitled this revision from [WIP][clang][extract-api] Add support for typedefs to [clang][extract-api] Add support for typedefs.Apr 4 2022, 5:54 AM
zixuw added inline comments.Apr 4 2022, 2:07 PM
clang/lib/ExtractAPI/DeclarationFragments.cpp
15

Empty line

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
628–635

Consider move the should-drop logic into SymbolGraphSerializer::shouldSkip(const APIRecord &Record) const so that we have a central place to see and manage which symbols get skipped.
This would also simplify things here as the filtering will automatically get handled in the following serializeAPIRecord(Record) line.

dang added inline comments.Apr 5 2022, 3:48 AM
clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
628–635

I need to know about it being a TypedefRecord so I can access the underlying type name, so I would need to attempt to dyn_cast in shouldSkip. Also this logic is specific to typedef records so I would prefer to keep it here.

dang updated this revision to Diff 420442.Apr 5 2022, 3:49 AM

Whitespace changes

dang marked an inline comment as done.Apr 5 2022, 3:50 AM
dang marked an inline comment as done.Apr 5 2022, 12:13 PM
zixuw added inline comments.Apr 5 2022, 12:33 PM
clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
641

Curious: where does this come from the format spec? Is this required/correctly populated?
I see a type property in https://github.com/apple/swift-docc-symbolkit/blob/0a45209833f4a151212c1aa38e13cfc03b9462e4/openapi.yaml#L239-L242, but couldn't determine if it means the underlying type for a typedef.
cc. @QuietMisdreavus

clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
16

nit: empty line

clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h
37

nit: explicit?

dang updated this revision to Diff 420773.Apr 6 2022, 3:52 AM

Address code review feedback.

zixuw accepted this revision.Apr 6 2022, 10:10 AM

LGTM! 🚢

This revision is now accepted and ready to land.Apr 6 2022, 10:10 AM
dang updated this revision to Diff 420950.Apr 6 2022, 11:07 AM

Rebase on top of latest changes.

zixuw accepted this revision.Apr 6 2022, 11:09 AM
This revision was landed with ongoing or failed builds.Apr 6 2022, 11:16 AM
This revision was automatically updated to reflect the committed changes.