dang added inline comments.
================
Comment at: clang/lib/SymbolGraph/ExtractAPIConsumer.cpp:197
+ void recordEnumConstants(EnumRecord *EnumRecord,
+ const EnumDecl::enumerator_range Constants) {
----------------
Should this be static or in an anonymous namespace?
================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:228
const LangOptions &LangOpts) {
+ auto AddLangPrefix = [&LangOpts](StringRef S) -> std::string {
+ return (getLanguageName(LangOpts) + "." + S).str();
----------------
Nice!
================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:253
+ Kind["identifier"] = AddLangPrefix("enum.case");
+ Kind["displayName"] = "Enum Case";
+ break;
----------------
"Enumeration Case"
================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:257
+ Kind["identifier"] = AddLangPrefix("enum");
+ Kind["displayName"] = "Enum";
+ break;
----------------
"Enumeration"
================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:321
+ case RelationshipKind::MemberOf:
+ Relationship["kind"] = "memberOf";
+ break;
----------------
Since we are going to add more cases to this switch, would you mind doing
something along the lines of:
```
const char *KindString = "";
switch(Kind) {
default:
llvm_unreachable("Unhandled relationship kind in Symbol Graph!");
break;
case RelationshipKind::MemberOf:
KindString = "memberOf";
break;
}
Relationship["kind"] = KindString
```
or a method in the serializer for getting the string representation of the
relationship kind?
================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:343
+ if (!Enum)
+ return;
+
----------------
Quick design question: Do we want to be silently failing in these situations
(especially since this shouldn't be happening)? Let me know what you think.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121873/new/
https://reviews.llvm.org/D121873
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
