[PATCH] D69890: [clangd] Improve the output of rename tests where there are failures.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 6 06:26:57 PST 2019


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM! Many thanks, the new tests are much simpler to read!



================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:31
+  llvm::StringRef Code = Test.code();
+  for (const auto &R : Test.ranges()) {
+    unsigned StartOffset = llvm::cantFail(positionToOffset(Code, R.start));
----------------
NIT: `Test.llvm::Annotations::ranges()` will return ranges with offsets.
Should simplify the code.


================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:44
 TEST(RenameTest, SingleFile) {
-  struct Test {
-    const char* Before;
-    const char* After;
-  } Tests[] = {
+  const char *Tests[] = {
       // Rename function.
----------------
NIT: worth documenting that `^` points to the start position of the rename and ranges point to the identifier that is being renamed.


================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:44
 TEST(RenameTest, SingleFile) {
-  struct Test {
-    const char* Before;
-    const char* After;
-  } Tests[] = {
+  const char *Tests[] = {
       // Rename function.
----------------
ilya-biryukov wrote:
> NIT: worth documenting that `^` points to the start position of the rename and ranges point to the identifier that is being renamed.
NIT: maybe use `llvm::StringRef Tests[]` instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69890/new/

https://reviews.llvm.org/D69890





More information about the cfe-commits mailing list