[PATCH] D121873: [clang][extract-api] Add enum support

[Daniel Grumberg via Phabricator via cfe-commits]

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