[PATCH] D122611: [clang][extract-api] Add support for macros

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Still going through the declaration fragments builder and `ExtractAPIVisitor/Consumer/Action` changes, but it seems that the patch needs a rebase onto the latest main right now as it's missing several already landed changes.



================
Comment at: clang/include/clang/ExtractAPI/API.h:117
 
-  GlobalRecord(GVKind Kind, StringRef Name, StringRef USR, PresumedLoc Loc,
+  GlobalRecord(StringRef Name, GVKind Kind, StringRef USR, PresumedLoc Loc,
                const AvailabilityInfo &Availability, LinkageInfo Linkage,
----------------
Now that we are re-ordering it, would it look more organized if we push `GVKind Kind` further down so that the two `StringRef`s could be adjacent, and we also keep a common list of arguments (`Name, USR, Loc, Availability, ..., SubHeading`) the same across all kinds of records upfront? So maybe push to the end after `SubHeading` and before `Signature`.


================
Comment at: clang/include/clang/ExtractAPI/API.h:117
 
-  GlobalRecord(GVKind Kind, StringRef Name, StringRef USR, PresumedLoc Loc,
+  GlobalRecord(StringRef Name, GVKind Kind, StringRef USR, PresumedLoc Loc,
                const AvailabilityInfo &Availability, LinkageInfo Linkage,
----------------
zixuw wrote:
> Now that we are re-ordering it, would it look more organized if we push `GVKind Kind` further down so that the two `StringRef`s could be adjacent, and we also keep a common list of arguments (`Name, USR, Loc, Availability, ..., SubHeading`) the same across all kinds of records upfront? So maybe push to the end after `SubHeading` and before `Signature`.
Also it seems that it becomes a requirement for the `*Record` c'tors that `Name` should be the first parameter in order to use the `addTopLevelRecord` template, though that's hidden in an anonymous namespace in the implementation file.
How can we communicate or document this clearly for future extensions?


================
Comment at: clang/include/clang/ExtractAPI/API.h:195
+  MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
+                        DeclarationFragments DeclarationFragments)
+      : APIRecord(RK_MacroDefinition, Name, USR, Loc, AvailabilityInfo(),
----------------
Don't we also need sub-headings for macros?


================
Comment at: clang/include/clang/ExtractAPI/API.h:201
+    return Record->getKind() == RK_MacroDefinition;
+  }
+};
----------------
Need out-of-line virtual anchor as this struct inherits a virtual table.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122611/new/

https://reviews.llvm.org/D122611