[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:17
 #include "clang/ExtractAPI/API.h"
+#include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/Support/JSON.h"
----------------
Not needed


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }
----------------
What's the cost/would it worth it to wrap the `emplace_back`s and `pop_back`s of `PathComponentContext` in meaningful APIs like `enterPathComponentContext` and `exitPathComponentContext`?
That way the code is more self-explanatory and easier to read. It took me some time and mental resources to figure out why the `pop_back` is placed here.


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:520-532
   for (const auto &Constant : Record.Constants) {
     auto EnumConstant = serializeAPIRecord(*Constant);
+
+    PathComponentContext.pop_back();
+
     if (!EnumConstant)
       continue;
----------------
Same comment as above, would be nice to have clear APIs for entering and exiting path component contexts.


================
Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:657
   Root["symbols"] = std::move(Symbols);
-  Root["relationhips"] = std::move(Relationships);
+  Root["relationships"] = std::move(Relationships);
 
----------------
oops 😬 


================
Comment at: clang/test/ExtractAPI/macro_undefined.c:51
     {
+      "accessLevel": "public"m
       "declarationFragments": [
----------------
s/m/,/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045