dang added a comment.
LGTM with minor changes
================
Comment at: clang/include/clang/ExtractAPI/Serialization/SerializerBase.h:58
- virtual ~APISerializer() = default;
};
----------------
It would be nice to keep this as default, i.e.
```
~APISetVisitor() = default;
```
================
Comment at:
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:20
-#include "clang/ExtractAPI/API.h"
#include "clang/ExtractAPI/APIIgnoresList.h"
----------------
In LLVM we tend to explicitly include any header we use and we definitely use
the definitions ins API.h
================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:748
-void SymbolGraphSerializer::serializeSingleRecord(const APIRecord *Record) {
+void SymbolGraphSerializer::visitSingleRecord(const APIRecord *Record) {
switch (Record->getKind()) {
----------------
This isn't part of the visitation scheme but more of a way of serializing a
single record. I would prefer to keep this as `serializeSingleRecord`
================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:808
Object SymbolGraphSerializer::serialize() {
- // Serialize global variables in the API set.
- for (const auto &GlobalVar : API.getGlobalVariables())
- serializeGlobalVariableRecord(*GlobalVar.second);
-
- for (const auto &GlobalFunction : API.getGlobalFunctions())
- serializeGlobalFunctionRecord(*GlobalFunction.second);
-
- // Serialize enum records in the API set.
- for (const auto &Enum : API.getEnums())
- serializeEnumRecord(*Enum.second);
-
- // Serialize struct records in the API set.
- for (const auto &Struct : API.getStructs())
- serializeStructRecord(*Struct.second);
-
- // Serialize Objective-C interface records in the API set.
- for (const auto &ObjCInterface : API.getObjCInterfaces())
- serializeObjCContainerRecord(*ObjCInterface.second);
-
- // Serialize Objective-C protocol records in the API set.
- for (const auto &ObjCProtocol : API.getObjCProtocols())
- serializeObjCContainerRecord(*ObjCProtocol.second);
-
- for (const auto &Macro : API.getMacros())
- serializeMacroDefinitionRecord(*Macro.second);
-
- for (const auto &Typedef : API.getTypedefs())
- serializeTypedefRecord(*Typedef.second);
-
+ APISetVisitor::traverseAPISet();
return serializeCurrentGraph();
----------------
Does this need to be explicit like this? Would it not work to just call
`traverseAPISet();` since we would inherit it through inheritance.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151477/new/
https://reviews.llvm.org/D151477
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
