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

Daniel Grumberg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 29 05:53:57 PDT 2022


dang added inline comments.


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:245
+    if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+      SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+      SuperClass.USR = API.recordUSR(SuperClassDecl);
----------------
dang wrote:
> dang wrote:
> > zixuw wrote:
> > > dang wrote:
> > > > 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.
> > > Right but that only makes sense once we kill the AST before serialization right? I mean I'm happy to wrap these in `copyString` now if we know for sure that we're doing that in a later patch. But it feels beyond the scope of this particular patch. Especially that we'd also need to go through the previous code to copy the names of global records, enums, etc. anyway.
> > > 
> > > I feel like that the change to surface `APISet` and punt serialization should be separated out from the macro change. And we can wrap all the name strings in that patch so that it's a meaningful atomic commit.
> > Yeah sounds good!
> I have looked into it a bit more and the ASTContext will still be live at that point (it gets deallocated right after we will be doing the serialization) so this is fine.
I thought it would still make sense to split the macro patch in two so here we go: https://reviews.llvm.org/D122648


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