kadircet added inline comments.
================
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(
----------------
ilya-biryukov wrote:
> Maybe have two variables instead? Would format better and `HeaderPath` is
> arguably more readable than `Header.first`
Is it that important?
================
Comment at: unittests/clangd/IndexActionTests.cpp:225
+ ASSERT_TRUE(IndexFile.Sources);
+ auto Nodes = toMap(*IndexFile.Sources);
+
----------------
ilya-biryukov wrote:
> 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
because toMap doesn't modify the contents of the nodes(to make sure we don't
change behaviour during the process), and all the strings are still referring
to the keys of the graph. Therefore the graph needs to be kept alive.
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
