[PATCH] D91102: [clangd] Enhance Clangd rename testing coverage

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 10 00:07:27 PST 2020


hokein added a comment.

thanks for doing this!



================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:525
+
+TEST_F(WithinFileRenameTest, Member) {
+  Tests = {
----------------
I'm not sure about the motivation of having a separated `TEST_F`, are these tests special? These tests are similar to the tests in `TEST(RenameTest, WithinFileRename)`, I wonder why not just adding these tests there?



================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:530
+          int [[M^oo]];
+          void Baz() { [[M^oo]] = 1; }
+        };
----------------
nit: I think this case is already covered in `MemberExpr in macros` above.


================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:552
+
+      X::X() : [[a^]](), b() {}
+      )cpp",
----------------
IIUC, it tests constructor initializer? looks like it is covered in `CXXConstructor initializer list.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91102



More information about the cfe-commits mailing list