kbobyrev updated this revision to Diff 233048.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Add another test for `~^Foo` and rebase on top of master.


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

https://reviews.llvm.org/D71247

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -672,11 +672,103 @@
                           std::vector<Diag> Diagnostics) override {}
   } DiagConsumer;
   // rename is runnning on the "^" point in FooH, and "[[]]" ranges are the
-  // expcted rename occurrences.
+  // expected rename occurrences.
   struct Case {
     llvm::StringRef FooH;
     llvm::StringRef FooCC;
   } Cases [] = {
+    {
+      // Constructor.
+      R"cpp(
+        class [[Foo]] {
+          [[Foo^]]();
+          ~[[Foo]]();
+        };
+      )cpp",
+      R"cpp(
+        #include "foo.h"
+        [[Foo]]::[[Foo]]() {}
+        [[Foo]]::~[[Foo]]() {}
+
+        void func() {
+          [[Foo]] foo;
+        }
+      )cpp",
+    },
+    {
+      // Destructor (selecting before the identifier).
+      R"cpp(
+        class [[Foo]] {
+          [[Foo]]();
+          ^~[[Foo]]();
+        };
+      )cpp",
+      R"cpp(
+        #include "foo.h"
+        [[Foo]]::[[Foo]]() {}
+        [[Foo]]::~[[Foo]]() {}
+
+        void func() {
+          [[Foo]] foo;
+        }
+      )cpp",
+    },
+    {
+      // Destructor (selecting before the identifier).
+      R"cpp(
+        class [[Foo]] {
+          [[Foo]]();
+          ~^[[Foo]]();
+        };
+      )cpp",
+      R"cpp(
+        #include "foo.h"
+        [[Foo]]::[[Foo]]() {}
+        [[Foo]]::~[[Foo]]() {}
+
+        void func() {
+          [[Foo]] foo;
+        }
+      )cpp",
+    },
+    {
+      // Destructor (selecting within the identifier).
+      R"cpp(
+        class [[Foo]] {
+          [[Foo]]();
+          ~[[F^oo]]();
+        };
+      )cpp",
+      R"cpp(
+        #include "foo.h"
+        [[Foo]]::[[Foo]]() {}
+        [[Foo]]::~[[Foo]]() {}
+
+        void func() {
+          [[Foo]] foo;
+        }
+      )cpp",
+    },
+    {
+      // Destructor (selecting after the identifier).
+      R"cpp(
+        class [[Foo]] {
+          [[Foo]]();
+          virtual ~ /*~Foo?*/[[Foo^]]() {
+            int a = 4;
+          }
+        };
+      )cpp",
+      R"cpp(
+        #include "foo.h"
+        [[Foo]]::[[Foo]]() {}
+        [[Foo]]::~[[Foo]]() {}
+
+        void func() {
+          [[Foo]] foo;
+        }
+      )cpp",
+    },
     {
       // classes.
       R"cpp(
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -87,17 +87,36 @@
   // range of the Decl. This would avoid allowing rename on unrelated tokens.
   //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
   //                 // we don't attempt to trigger rename on this position.
-  // FIXME: Make this work on destructors, e.g. "~F^oo()".
   if (const auto *D = SelectedNode->ASTNode.get<Decl>()) {
-    if (D->getLocation() != TokenStartLoc)
-      return {};
+    if (D->getLocation() != TokenStartLoc) {
+      // Destructor->getLocation() points to ~. In this case, TokenStartLoc
+      // should point to the next token.
+      const auto *Destructor = llvm::dyn_cast<CXXDestructorDecl>(D);
+      if (!Destructor)
+        return {};
+      // There should be exactly two tokens within inspected range: tok::tilde
+      // and tok::identifier.
+      const auto Tokens = AST.getTokens().expandedTokens(
+          {Destructor->getLocation(), TokenStartLoc});
+      if (Tokens.size() != 2 || Tokens.back().kind() != tok::identifier)
+        return {};
+    }
   }
 
   llvm::DenseSet<const Decl *> Result;
   for (const auto *D :
        targetDecl(SelectedNode->ASTNode,
-                  DeclRelation::Alias | DeclRelation::TemplatePattern))
-    Result.insert(D);
+                  DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+    // If the cursor is at the underlying CXXRecordDecl of the
+    // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
+    // get the primary template maunally.
+    // FIXME: Do proper and well-defined canonicalization.
+    D = D->getDescribedTemplate() ? D->getDescribedTemplate() : D;
+    const auto *ND = llvm::dyn_cast<NamedDecl>(D);
+    // Get to CXXRecordDecl from constructor or destructor.
+    ND = tooling::getCanonicalSymbolDeclaration(ND);
+    Result.insert(ND);
+  }
   return Result;
 }
 
@@ -214,17 +233,11 @@
 // Return all rename occurrences in the main file.
 std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
                                                       const NamedDecl &ND) {
-  // In theory, locateDeclAt should return the primary template. However, if the
-  // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
-  // will be the CXXRecordDecl, for this case, we need to get the primary
-  // template maunally.
-  const auto &RenameDecl =
-      ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
   // getUSRsForDeclaration will find other related symbols, e.g. virtual and its
   // overriddens, primary template and all explicit specializations.
   // FIXME: Get rid of the remaining tooling APIs.
-  std::vector<std::string> RenameUSRs = tooling::getUSRsForDeclaration(
-      tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext());
+  std::vector<std::string> RenameUSRs =
+      tooling::getUSRsForDeclaration(&ND, AST.getASTContext());
   llvm::DenseSet<SymbolID> TargetIDs;
   for (auto &USR : RenameUSRs)
     TargetIDs.insert(SymbolID(USR));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to