dang requested changes to this revision.
dang added a comment.
This revision now requires changes to proceed.
Great start but there are still some rough edges to polish!
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:19
#include "clang/ExtractAPI/APIIgnoresList.h"
+#include "llvm/Support/JSON.h"
#include "llvm/Support/raw_ostream.h"
----------------
This shouldn't be needed at this point.
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:28-31
+struct APISetVisitorOption {
/// Do not include unnecessary whitespaces to save space.
bool Compact;
};
----------------
This is very specific to SymbolGraphSerializer or at last to JSON.
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:37
+ /// Traverse the API information to output to \p os.
+ void traverseAPISet(raw_ostream &os){};
+
----------------
This shouldn't be needed anymore.
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:84
+ for (const auto &Protocol : API.getObjCProtocols())
+ getDerived()->visitObjCContainerRecord(*Protocol.second);
+ }
----------------
This should visit some sort Protocol Records specifically. Likewise for
interfaces.
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:100
+ /// Visit a global variable record.
+ void visitGlobalVariableRecord(const GlobalVariableRecord &Record);
+
----------------
This should have a default implementation.
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:138
protected:
- APISerializer(const APISet &API, const APIIgnoresList &IgnoresList,
- APISerializerOption Options = {})
+ APISetVisitor(const APISet &API, const APIIgnoresList &IgnoresList,
+ APISetVisitorOption Options = {})
----------------
I think that `APIIgnoresList` Should be pushed down into the
SymbolGraphSerializer.
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:142
- virtual ~APISerializer() = default;
+ virtual ~APISetVisitor() = default;
+ Derived *getDerived() { return static_cast<Derived *>(this); };
----------------
Don't need a virtual destructor anymore.
================
Comment at:
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:12
///
-/// Implement an APISerializer for the Symbol Graph format for ExtractAPI.
+/// Implement an APISetVisitor for the Symbol Graph format for ExtractAPI.
/// See https://github.com/apple/swift-docc-symbolkit.
----------------
================
Comment at:
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:94
/// Just serialize the currently recorded objects in Symbol Graph format.
- Object serializeCurrentGraph();
+ Object visitCurrentGraph();
----------------
I think these should still be called serializeXXX as they aren't part of the
visitation interface and are very specific to the symbol graph format.
================
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);
----------------
I think we shouldn't need to have this method. APISetVisitor should handle
calling the relevant visitMethods for the members.
================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:221
/// references, and the interface language name.
-Object serializeIdentifier(const APIRecord &Record, Language Lang) {
+Object visitIdentifier(const APIRecord &Record, Language Lang) {
Object Identifier;
----------------
I think all these helper methods for serializing specific chunks of symbol
graphs should still be names serialize
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
