[PATCH] D135489: [clangd] Fix rename for symbol introduced by UsingDecl

[Tom Praschan via Phabricator via cfe-commits]

tom-anders updated this revision to Diff 466292.
tom-anders added a comment.

Add additional tests, only handle UsingDecl instead of BaseUsingDecl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135489

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
@@ -817,6 +817,29 @@
           char [[da^ta]];
         } @end
       )cpp",
+
+      // Issue 170: Rename symbol introduced by UsingDecl
+      R"cpp(
+        namespace ns { void [[f^oo]](); } 
+
+        using ns::[[f^oo]];
+
+        void f() {
+            [[f^oo]]();
+            auto p = &[[f^oo]];
+        }
+      )cpp",
+
+      // Issue 170: using decl that imports multiple overloads
+      // -> Only the overload under the cursor is renamed
+      R"cpp(
+        namespace ns { int [[^foo]](int); char foo(char); }
+        using ns::[[foo]];
+        void f() {
+          [[^foo]](42);
+          foo('x');
+        }
+      )cpp",
   };
   llvm::StringRef NewName = "NewName";
   for (llvm::StringRef T : Tests) {
@@ -1062,6 +1085,14 @@
         };
       )cpp",
        "no symbol", false},
+
+      {R"cpp(// FIXME we probably want to rename both overloads here,
+             // but renaming currently assumes there's only a 
+             // single canonical declaration.
+        namespace ns { int foo(int); char foo(char); }
+        using ns::^foo;
+      )cpp",
+       "there are multiple symbols at the given location", !HeaderFile},
   };
 
   for (const auto& Case : Cases) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -133,6 +133,10 @@
     if (const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember())
       return canonicalRenameDecl(OriginalVD);
   }
+  if (const auto *UD = dyn_cast<UsingShadowDecl>(D)) {
+    if (const auto* TargetDecl = UD->getTargetDecl()) 
+      return canonicalRenameDecl(TargetDecl);
+  }
   return dyn_cast<NamedDecl>(D->getCanonicalDecl());
 }
 
@@ -157,6 +161,21 @@
   return Result;
 }
 
+// For something like 
+//     namespace ns { void foo(); }
+//     void bar() { using ns::f^oo; foo(); }
+// locateDeclAt() will return a UsingDecl and foo's actual declaration.
+// For renaming, we're only interested in foo's declaration, so drop the other one
+void filterUsingDecl(llvm::DenseSet<const NamedDecl *>& Decls) {
+  // There should never be more than one UsingDecl here, 
+  // otherwise the rename would be ambiguos anyway.
+  auto UD = std::find_if(Decls.begin(), Decls.end(), 
+                         [](const NamedDecl* D) { return llvm::isa<UsingDecl>(D); });
+  if (UD != Decls.end()) {
+    Decls.erase(UD);
+  }
+}
+
 // By default, we exclude symbols from system headers and protobuf symbols as
 // renaming these symbols would change system/generated files which are unlikely
 // to be good candidates for modification.
@@ -737,6 +756,7 @@
     return makeError(ReasonToReject::UnsupportedSymbol);
 
   auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
+  filterUsingDecl(DeclsUnderCursor);
   if (DeclsUnderCursor.empty())
     return makeError(ReasonToReject::NoSymbolFound);
   if (DeclsUnderCursor.size() > 1)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135489.466292.patch
Type: text/x-patch
Size: 3249 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221008/a0ceb547/attachment-0001.bin>