[PATCH] D152356: [clang][ExtractAPI] Add --emit-symbol-graph option

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.


================
Comment at: clang/include/clang/ExtractAPI/ExtractAPIActionBase.h:25
+///
+/// Deriving from this class equipts an action with all the necessary tools to
+/// generate ExractAPI information in form of symbol-graphs
----------------



================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:227
 
+struct BasicExtractAPIVisitor : ExtractAPIVisitor<BasicExtractAPIVisitor> {
+  BasicExtractAPIVisitor(ASTContext &Context, APISet &API)
----------------
I don't think this is needed `ExtractAPIVisitor` was written so that it could be used as is. If it isn't then it's a bug we should fix.


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:255
 
+class SymbolGraphConsumer : public ASTConsumer {
+public:
----------------
Not sure I like the name `SymbolGraphConsumer`, but I don't have a great suggestion, maybe `WrappingExtractAPIConsumer` so that at least it's clear that it is intended to be used by `WrappingExtractAPIAction`?


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:285
 
-class MacroCallback : public PPCallbacks {
+class MacroCallBack : public PPCallbacks {
 public:
----------------
the name change here is unnecessary


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:567-575
+    bool IsQuoted = false;
+    for (const FrontendInputFile &FIF : Inputs) {
+      StringRef FilePath = FIF.getFile();
+      if (auto RelativeName = getRelativeIncludeName(CI, FilePath, &IsQuoted))
+        KnownInputFiles.emplace_back(
+            static_cast<SmallString<32>>(*RelativeName), IsQuoted);
+      else
----------------
I don't think we need to do all this computation since we just serialize symbol graphs for all symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152356