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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
