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

[Tom Praschan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac21938fbdfa: [clangd] Fix rename for symbol introduced by UsingDecl (authored by tom-anders).

Changed prior to commit:
  https://reviews.llvm.org/D135489?vs=466292&id=466360#toc

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,22 @@
   return Result;
 }
 
+void filterRenameTargets(llvm::DenseSet<const NamedDecl *> &Decls) {
+  // 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. 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 +757,7 @@
     return makeError(ReasonToReject::UnsupportedSymbol);
 
   auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
+  filterRenameTargets(DeclsUnderCursor);
   if (DeclsUnderCursor.empty())
     return makeError(ReasonToReject::NoSymbolFound);
   if (DeclsUnderCursor.size() > 1)


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