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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Everything looks good, just a single important request about testing the included files do not have any edges in the resulting graph



================
Comment at: clangd/index/Background.cpp:79
+
+  // Since the strings in direct includes are references to the keys of the
+  // fullgraph, we need to create entries in our new sub-graph for those
----------------
NIT: the comment could probably be simplified to something like `URIs inside nodes must point into the keys of the same IncludeGraph`


================
Comment at: clangd/index/Background.cpp:386
 
-  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(),
----------------
"sources" might be a bit confusing. I'd probably use "files" instead, but that doesn't look ideal too.


================
Comment at: unittests/clangd/BackgroundIndexTests.cpp:197
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h
----------------
Maybe check the full shape of the graph? E.g. also add a check that `A.h` does not have any includes and its digest is populated.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062