[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.


================
Comment at: unittests/clangd/CodeCompleteTests.cpp:289
+    };
+    void Foo::pub() { this->^ }
+  )cpp");
----------------
ilya-biryukov wrote:
> The `^` symbol conflicts with the corresponding operator.
> Even though it's not widely used, I'm wondering whether we should use a different marker for completion position.
Almost all characters conflict with something in C++ :-(
^ is rarely going to cause problems and is suggestive of an insertion point.

The failure modes here don't seem worth worrying about:
 - we add a test that needs to use `^`. The assert fires, it's easy to tell what happened, and we have to add escaping or change the notation. Annoying, but easy to detect and pretty unlikely.
 - we add a test that needs to use exactly one `^`, *and* we forget to add the cursor marker to the test. The assert doesn't fire, the test fails in some random way. This is really annoying, but also vanishingly unlikely.

(I'd feel differently if this wasn't just a local test helper of course)



================
Comment at: unittests/clangd/CodeCompleteTests.cpp:335
+  )cpp",
+                             Opts);
+  EXPECT_THAT(Results.items,
----------------
ilya-biryukov wrote:
> The formatting is a bit weird. Is this a `clang-format` bug?
Weird formatting is a clang-format *feature*.
I reformatted all the tests, now they're weird in a different way.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40952