ilya-biryukov added inline comments.
================
Comment at: clangd/Headers.h:64
+// Important: The graph generated by those callbacks might contain cycles and
+// self edges.
using IncludeGraph = llvm::StringMap<IncludeGraphNode>;
----------------
And multi-edges too, right? Even though they're not useful.
================
Comment at: unittests/clangd/IndexActionTests.cpp:168
+ std::string MainFilePath = testPath("main.cpp");
+ std::pair<std::string, std::string> CommonHeader = {testPath("common.h"),
+ R"cpp(
----------------
Maybe have two variables instead? Would format better and `HeaderPath` is
arguably more readable than `Header.first`
================
Comment at: unittests/clangd/IndexActionTests.cpp:211
+ std::string MainCode = R"cpp(
+ #ifndef FOO
+ #define FOO "main.cpp"
----------------
Is there a way to make this format nicer?
================
Comment at: unittests/clangd/IndexActionTests.cpp:225
+ ASSERT_TRUE(IndexFile.Sources);
+ auto Nodes = toMap(*IndexFile.Sources);
+
----------------
Why not `auto Nodes = toMap(*runIndexingAction(MainFilePath).Sources)`?
Optional has an assertion, so we'll catch empty results anyway, no need to make
the test more verbose
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54999/new/
https://reviews.llvm.org/D54999
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
