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

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.


================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:321
+  case RelationshipKind::MemberOf:
+    Relationship["kind"] = "memberOf";
+    break;
----------------
dang wrote:
> 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?
Definitely! That's a good idea. I'm going to have a 'getString' sort of method in the serializer to do this.


================
Comment at: clang/lib/SymbolGraph/Serialization.cpp:343
+  if (!Enum)
+    return;
+
----------------
dang wrote:
> 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.
I'm using this check to intentionally skip symbols that we do not want to spit out, for example unconditionally unavailable symbols, or after we have support for typedef records, anonymous enum decls that's declared with a `typedef` so that we don't have duplicate information, etc.
`Optional<Object> serializeAPIRecord` does this check now, and if we `Serializer::shouldSkip` it, `None` will be returned. So it is expected, not really silently failing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121873/new/

https://reviews.llvm.org/D121873