evelez7 abandoned this revision. evelez7 added inline comments.
================ Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:28-31 +struct APISetVisitorOption { /// Do not include unnecessary whitespaces to save space. bool Compact; }; ---------------- dang wrote: > This is very specific to SymbolGraphSerializer or at last to JSON. If there aren't any base visitor options, I'll just move this to `SymbolGraphSerializer` without any inheritance. ================ Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:84 + for (const auto &Protocol : API.getObjCProtocols()) + getDerived()->visitObjCContainerRecord(*Protocol.second); + } ---------------- dang wrote: > This should visit some sort Protocol Records specifically. Likewise for > interfaces. Not sure I understand, these functions essentially replace the loops in the old `SymbolGraphSerializer::serialize` loops, which are now grouped as function calls in `APISetVisitor::traverseAPISet`. Should I add functions that take Protocols, Interfaces as parameters to visit them specifically? ================ Comment at: clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:129 template <typename MemberTy> - void serializeMembers(const APIRecord &Record, - const SmallVector<std::unique_ptr<MemberTy>> &Members); + void visitMembers(const APIRecord &Record, + const SmallVector<std::unique_ptr<MemberTy>> &Members); ---------------- dang wrote: > I think we shouldn't need to have this method. APISetVisitor should handle > calling the relevant visitMethods for the members. I've been considering how to move it to `APISetVisitor`. Its current implementation doesn't seem like it would fit well with a generic base implementation. It would be overridden to call `visitRelationship` which relies on a `RelationshipKind` parameter. Since that's SymbolKit specific, I don't think we'd raise that to the base class. If there was a base relationship struct that `RelationshipKind` inherited from, it might solve that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151293/new/ https://reviews.llvm.org/D151293 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits