[PATCH] D121873: [clang][extract-api] Add enum support

dexonsmith added inline comments. Comment at: clang/test/ExtractAPI/enum.c:11 +// RUN: %t/output.json >> %t/output-normalized.json +// RUN: diff %t/reference.output.json %t/output-normalized.json + Diffing against reference output is usually not ideal: - It

[PATCH] D121873: [clang][extract-api] Add enum support

dexonsmith added inline comments. Comment at: clang/test/ExtractAPI/enum.c:5 +// RUN: %t/reference.output.json +// RUN: %clang -extract-api -target arm64-apple-macosx \ +// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s I just noticed these tests

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw added a comment. In D121873#3403082 , @mstorsjo wrote: > This (or some closely related commit?) causes a huge amount of warnings when > building with GCC: > > warning: ‘clang::extractapi::EnumRecord’ has a field >

[PATCH] D121873: [clang][extract-api] Add enum support

mstorsjo added a comment. This (or some closely related commit?) causes a huge amount of warnings when building with GCC: warning: ‘clang::extractapi::EnumRecord’ has a field ‘clang::extractapi::EnumRecord::Constants’ whose type uses the anonymous namespace [-Wsubobject-linkage] 167 |

[PATCH] D121873: [clang][extract-api] Add enum support

This revision was automatically updated to reflect the committed changes. Closed by commit rG71b4c22612a0: [clang][extract-api] Add enum support (authored by zixuw). Changed prior to commit: https://reviews.llvm.org/D121873?vs=417424=417667#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D121873: [clang][extract-api] Add enum support

dang accepted this revision. dang added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121873/new/ https://reviews.llvm.org/D121873 ___

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw updated this revision to Diff 417424. zixuw added a comment. Rebase on main with changed and merged refactoring in D122160 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121873/new/

[PATCH] D121873: [clang][extract-api] Add enum support

dang added inline comments. Comment at: clang/include/clang/ExtractAPI/API.h:33 +namespace { + I still think this belongs in `APISet`. It belongs there because it defines unique pointers that are specifically tied to the lifetime of `APISet`. Any other thing

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw added inline comments. Comment at: clang/lib/SymbolGraph/ExtractAPIConsumer.cpp:197 + void recordEnumConstants(EnumRecord *EnumRecord, + const EnumDecl::enumerator_range Constants) { dang wrote: > Should this be static or in an

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw updated this revision to Diff 417154. zixuw marked 4 inline comments as done. zixuw added a comment. - Address review issues; - Fix memory managment for EnumConstantRecord and EnumRecord Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw updated this revision to Diff 417147. zixuw marked 2 inline comments as done. zixuw added a comment. Rebase onto D122160 refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121873/new/

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw added a comment. Herald added a subscriber: StephenFan. Hi Sam! Thanks for your interest and taking a look! In D121873#3391059 , @sammccall wrote: > Hey, apologies for missing the initial patch in D119479 > . > >

[PATCH] D121873: [clang][extract-api] Add enum support

dang added inline comments. Comment at: clang/lib/SymbolGraph/Serialization.cpp:343 + if (!Enum) +return; + zixuw wrote: > dang wrote: > > Quick design question: Do we want to be silently failing in these > > situations (especially since this shouldn't be

[PATCH] D121873: [clang][extract-api] Add enum support

sammccall added a comment. Hey, apologies for missing the initial patch in D119479 . This sounds really interesting! The lack of docs make it very difficult to understand what this library is for and how to use it if you don't already know. It's not clear what

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw added inline comments. Comment at: clang/lib/SymbolGraph/Serialization.cpp:321 + case RelationshipKind::MemberOf: +Relationship["kind"] = "memberOf"; +break; dang wrote: > Since we are going to add more cases to this switch, would you mind doing

[PATCH] D121873: [clang][extract-api] Add enum support

dang added inline comments. Comment at: clang/lib/SymbolGraph/ExtractAPIConsumer.cpp:197 + void recordEnumConstants(EnumRecord *EnumRecord, + const EnumDecl::enumerator_range Constants) { Should this be static or in an anonymous

[PATCH] D121873: [clang][extract-api] Add enum support

zixuw created this revision. Herald added a reviewer: dang. Herald added a project: All. zixuw requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add support for enum records - Add `EnumConstantRecord` and `EnumRecord` to store API