[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: unittests/clangd/TestTU.h:41
 
+  static TestTU withAllCode(llvm::StringRef HeaderCode, llvm::StringRef Code,
+                            llvm::StringRef Filename = "") {
----------------
hokein wrote:
> sammccall wrote:
> > We've avoided adding this in the past because it's less readable. Please assign the fields separately instead.
> I'm tempted to keep it here, while it's less readable, it does more things, and can make client code more readable (see newly-added tests), otherwise we have to repeat these statements in a few places.
That's what I mean, I find the newly-added tests hard to read, because "allcode" isn't clear (particularly when one of the params is a filename) and the order of parameters is not obvious.
I think they would be much clearer with the fields assigned individually as the existing tests do.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279