[PATCH] D152770: [clang][ExtractAPI] Add support for Objective-C categories

[R4444 via Phabricator via cfe-commits]

Ruturaj4 added inline comments.


================
Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:488
+  bool IsFromExternalModule = true;
+  for (const auto &Interface : API.getObjCInterfaces()) {
+    if (InterfaceDecl->getName() == Interface.second.get()->Name) {
----------------
dang wrote:
> I think this is fine for now but there must be a better way of doing this that doesn't involve traversing the whole APISet
I looked at different api methods, but still not able to figure it out. I will do it once this gets approved. In a following patch.


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:219
   Object Identifier;
-  Identifier["precise"] = Record.USR;
+  if (auto *CategoryRecord =
+          dyn_cast_or_null<const ObjCCategoryRecord>(&Record))
----------------
dang wrote:
> I don't think this is right. My expectation is that category records that need to get emitted would still have their own unique USR.
Thanks. I removed the check.


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:759
+  // Check if the current Category Record has been visited before, if not add.
+  if (!(visitedCategories.contains(Record.Interface.Name) > 0)) {
+    visitedCategories.insert(Record.Interface.Name);
----------------
dang wrote:
> What happens when there are multiple categories to the same type though?
Yes! I added this check keeping that in mind. If the parent symbol of the category is not added before then it pushes it to serialize (but skips if already present). Otherwise, just adds the category.

For e.g. in example test - objc_various_categories.m, there are two categories extending the same type:

```
@interface NSString (Category1)
-(void) StringMethod;
@end

@interface NSString (Category2)
-(void) StringMethod2;
@end
```

In this case, it adds the symbol for NSString (parent of categories `Category1` and `Category2`). I fixed the comments to make that clear.


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:775
+  Relationship["targetFallback"] = Record.Interface.Name;
+  Relationship["kind"] = getRelationshipString(RelationshipKind::MemberOf);
+  Relationships.emplace_back(std::move(Relationship));
----------------
dang wrote:
> I would expect this to be a new relationship kind that reads as "extensionTo"
Thanks, that makes sense. I fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152770