[PATCH] D78048: [clangd] Add tests that no-op changes are cheap

[Sam McCall via Phabricator via cfe-commits]

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