sammccall marked 3 inline comments as done.
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/ClangdTests.cpp:501
+MATCHER_P4(Stats, Name, UsesMemory, PreambleBuilds, ASTBuilds, "") {
+ return arg.first() == Name && (arg.second.UsedBytes != 0) == UsesMemory &&
+ arg.second.PreambleBuilds == PreambleBuilds &&
----------------
kadircet wrote:
> nit:
>
> ```
> std::make_tuple(arg.first(), arg.second.UsedBytes != 0,
> arg.second.PreambleBuilds, arg.second.ASTBuilds) == std::tie(Name,
> UsesMemory, PreambleBuilds, ASTBuilds);
> ```
Switched to tie for the trivial fields, but I don't like separating out the
unnamed (first()) or nonobvious (UsedBytes!=0) things from what they're
compared to.
================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:762
+ ASSERT_TRUE(DoUpdate(SourceContents));
+ ASSERT_FALSE(DoUpdate(SourceContents));
+ ASSERT_EQ(S.fileStats().lookup(Source).ASTBuilds, 4u);
----------------
kadircet wrote:
> nit: maybe we drop these noop updates except the first one.
I think that allows an implementation that always compares the compile command
to the first-ever (i.e. changes to the compile command don't "stick")
This is the primary test that verifies that no-op changes are recognized, so I
think we should keep testing that (it's pretty cheap)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78048/new/
https://reviews.llvm.org/D78048
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
