[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/index/Background.cpp:70
+// We keep only the node "Path" and its edges.
+IncludeGraph getSubGraph(const URI &Uri, const IncludeGraph &FullGraph) {
+  IncludeGraph IG;
----------------
Naming: technically the variable name should be `URI`, but to avoid clashing with the type name we could use `U` 



================
Comment at: clangd/index/Background.cpp:259
     auto RS = llvm::make_unique<RefSlab>(std::move(Refs).build());
+    std::unique_ptr<IncludeGraph> IG =
+        Index.Sources ? llvm::make_unique<IncludeGraph>(getSubGraph(
----------------
Maybe use auto? The type is spelled explicitly in the rhs


================
Comment at: clangd/index/Background.cpp:260
+    std::unique_ptr<IncludeGraph> IG =
+        Index.Sources ? llvm::make_unique<IncludeGraph>(getSubGraph(
+                            URI::create(Path), Index.Sources.getValue()))
----------------
What are the cases when it happens in practice? Do we ever run the indexer without producing the include graph?


================
Comment at: clangd/index/Background.cpp:388
 
-  log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
-      Symbols.size(), Refs.numRefs());
-  SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
-  SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
-  IndexFileIn Index;
-  Index.Symbols = std::move(Symbols);
-  Index.Refs = std::move(Refs);
+  log("Indexed {0} ({1} symbols, {2} refs {3} sources)",
+      Inputs.CompileCommand.Filename, Index.Symbols->size(),
----------------
add a comma before `{3} sources`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062