[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:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }
----------------
QuietMisdreavus wrote:
> zixuw wrote:
> > 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.
> What's the use of having the `emplace_back` call inside `serializeAPIRecord` but to pop it outside? It seems like it's easier to mess up for new kinds of records.
The reason to `emplace_back` the path component inside `serializeAPIRecord` is that the `pathComponent` field of the `Record` is serialized in there so you want to include the name of the symbol itself in the path component list.
The `pop_back` is delayed til all other additional serialization for a specific kind of record, for example `serializeEnumRecord` handles all the enum cases after processing the enum record itself using `serializeAPIRecord`. So in order to correctly include the enum name in the path components of all the enum cases, the enum name has to stay in `PathComponentContext` until all members are serialized.

This is exactly the reason why I wanted a clearer API to make it easier to see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123045

STAMPS
actor(@zixuw) application(Differential) author(@dang) herald(H423) herald(H576) herald(H864) herald(H875) herald(H876) monogram(D123045) object-type(DREV) phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) via(web)