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

2022-04-06 Thread Daniel Grumberg via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG28d793144f2a: [clang][extract-api] Fix small issues with SymbolGraphSerializer (authored by dang). Repository: rG LLVM Github Monorepo CHANGES SI

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

2022-04-06 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 420938. dang added a comment. Remove unnecessary include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123045/new/ https://reviews.llvm.org/D123045 Files: clang/include/clang/ExtractAPI/Serialization/SymbolGra

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

2022-04-06 Thread Zixu Wang via Phabricator via cfe-commits
zixuw accepted this revision. zixuw added a comment. Unused include of declaration fragments in SymbolGraphSerializer. Otherwise LGTM! Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:17 #include "clang/ExtractAPI/API.h" +#include "clang/ExtractAPI/Dec

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

2022-04-06 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 420883. dang marked an inline comment as done. dang added a comment. Make `SymbolGraphSerializer::serializeAPIRecord` const again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123045/new/ https://reviews.llvm.org

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

2022-04-06 Thread Victoria Mitchell via Phabricator via cfe-commits
QuietMisdreavus accepted this revision. QuietMisdreavus added a comment. This revision is now accepted and ready to land. I like the idea of using the RAII context/guard to manage the path components stack. I have one non-blocking comment about the rest of the patch now. Comme

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

2022-04-06 Thread Daniel Grumberg via Phabricator via cfe-commits
dang marked an inline comment as done. dang added inline comments. Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510 Symbols.emplace_back(std::move(*Obj)); + PathComponentContext.pop_back(); } QuietMisdreavus wrote: > dang wrote: >

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

2022-04-06 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 420770. dang added a comment. Manage PatchComponents stack manipulation using RAII to avoid forgetting to pop the stack when returning early from `serializeAPIRecord`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

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

2022-04-05 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 420598. dang added a comment. Add descriptive method names for manipulating the path component stack. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123045/new/ https://reviews.llvm.org/D123045 Files: clang/incl

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

2022-04-05 Thread Victoria Mitchell via Phabricator via cfe-commits
QuietMisdreavus added inline comments. Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510 Symbols.emplace_back(std::move(*Obj)); + PathComponentContext.pop_back(); } dang wrote: > dang wrote: > > zixuw wrote: > > > zixuw wrote: > >

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

2022-04-05 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 420451. dang added a comment. Address code review feedback: - Fix the function signature serialization - Typo fix of "m to ", Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123045/new/ https://reviews.llvm.org/D12

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

2022-04-05 Thread Daniel Grumberg via Phabricator via cfe-commits
dang added inline comments. Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510 Symbols.emplace_back(std::move(*Obj)); + PathComponentContext.pop_back(); } dang wrote: > zixuw wrote: > > zixuw wrote: > > > QuietMisdreavus wrote: > >

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

2022-04-05 Thread Daniel Grumberg via Phabricator via cfe-commits
dang marked 2 inline comments as done. dang added a comment. In D123045#3428030 , @QuietMisdreavus wrote: > In D123045#3427992 , @zixuw wrote: > >> In D123045#3427699 ,

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

2022-04-04 Thread Victoria Mitchell via Phabricator via cfe-commits
QuietMisdreavus added a comment. In D123045#3427992 , @zixuw wrote: > In D123045#3427699 , > @QuietMisdreavus wrote: > >> After a quick scan comparing the current output of these symbol graphs with >> the primar

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

2022-04-04 Thread 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(); } zixuw wrote: > QuietMisdreavus wrote: > > zixuw wrote: > > > What's the cost

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

2022-04-04 Thread 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

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

2022-04-04 Thread Zixu Wang via Phabricator via cfe-commits
zixuw added a comment. In D123045#3427699 , @QuietMisdreavus wrote: > After a quick scan comparing the current output of these symbol graphs with > the primary library used for reading them > , the last th

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

2022-04-04 Thread Victoria Mitchell via Phabricator via cfe-commits
QuietMisdreavus added a comment. After a quick scan comparing the current output of these symbol graphs with the primary library used for reading them , the last thing i can spot that's "off" is that the "function signature" is currently being seri

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

2022-04-04 Thread 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 a

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

2022-04-04 Thread Daniel Grumberg via Phabricator via cfe-commits
dang created this revision. dang added reviewers: zixuw, ributzka, QuietMisdreavus. Herald added a project: All. dang requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This includes: - replacing "relationhips" with "relationships" - emitting