[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

Daniel Grumberg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 28 13:37:27 PDT 2022


dang added inline comments.


================
Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:624-660
 FunctionSignature
 DeclarationFragmentsBuilder::getFunctionSignature(const FunctionDecl *Func) {
   FunctionSignature Signature;
 
   for (const auto *Param : Func->parameters()) {
     StringRef Name = Param->getName();
     DeclarationFragments Fragments = getFragmentsForParam(Param);
----------------
If I am not mistaken, these do the same thing but need separate overloads because the types don't share a common super class. If this is true we should still remove the duplication here. There are two ways of doing this AFAIK:
1. Have a template helper function that does the work and call it from both overloads
2. Create a "union" type that contains either of the two alternatives and delegates the method calls appropriately.


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:245
+    if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+      SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+      SuperClass.USR = API.recordUSR(SuperClassDecl);
----------------
Shouldn't we be recording this string in the StringAllocator for reuse later. I have a patch in progress for macro support that will do the serialization once the AST isn't available anymore so we really shouldn't be taking in StringRefs for stuff in the AST.


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:455
+    for (const auto *Protocol : Protocols)
+      Container->Protocols.emplace_back(Protocol->getName(),
+                                        API.recordUSR(Protocol));
----------------
I think we need to allocate these string in the StringAllocator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122446



More information about the cfe-commits mailing list