[rename] Don't overwrite the template argument when renaming a template function.

[ioeric]

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

227 ↗

(On Diff #119645)

I wonder what would happen if we have foo <int>()?

[hokein]

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

227 ↗

(On Diff #119645)

There will be a blank left like bar <int>(); if we don't format the apply replacement, but I don't think this case really matters because we often format the placements before applying them.

[ioeric]

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

227 ↗

(On Diff #119645)

Could you add a test for this? Thanks!

[hokein]

Test for foo <int>().

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

227 ↗

(On Diff #119645)

Done, but note that our unittest will format the expected/actual code before doing verification.

[ioeric]

lgtm

[cierpuchaw]

unittests/Rename/RenameFunctionTest.cpp

232 ↗

(On Diff #119654)

Shouldn't this be namespace nb {?

[hokein]

unittests/Rename/RenameFunctionTest.cpp

232 ↗

(On Diff #119654)

It is intended. We don't change the namespace here, only the name of the definition will be changed.

cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
     const NamedDecl *Decl = Expr->getFoundDecl();
     // Get the underlying declaration of the shadow declaration introduced by a
     // using declaration.
     if (auto *UsingShadow = llvm::dyn_cast<UsingShadowDecl>(Decl)) {
       Decl = UsingShadow->getTargetDecl();
     }
 
     auto StartLoc = Expr->getLocStart();
-    auto EndLoc = Expr->getLocEnd();
+    // For template function call expressions like `foo<int>()`, we want to
+    // restrict the end of location to just before the `<` character.
+    SourceLocation EndLoc = Expr->hasExplicitTemplateArgs()
+                                ? Expr->getLAngleLoc().getLocWithOffset(-1)
+                                : Expr->getLocEnd();
+
     // In case of renaming an enum declaration, we have to explicitly handle
     // unscoped enum constants referenced in expressions (e.g.
     // "auto r = ns1::ns2::Green" where Green is an enum constant of an unscoped
     // enum decl "ns1::ns2::Color") as these enum constants cannot be caught by
     // TypeLoc.
     if (const auto *T = llvm::dyn_cast<EnumConstantDecl>(Decl)) {
       // FIXME: Handle the enum constant without prefix qualifiers (`a = Green`)
       // when renaming an unscoped enum declaration with a new namespace.

cfe/trunk/unittests/Rename/RenameFunctionTest.cpp

std::string Expected = R"(
       namespace na {
         void Y();
         void Y() {}
       })";
   std::string After = runClangRenameOnCode(Before, "na::X", "na::Y");
   CompareSnippets(Expected, After);
 }
 
+TEST_F(RenameFunctionTest, RenameTemplateFunctions) {
+  std::string Before = R"(
+      namespace na {
+      template<typename T> T X();
+      }
+      namespace na { void f() { X<int>(); } }
+      namespace nb { void g() { na::X          <int>(); } }
+      )";
+  std::string Expected = R"(
+      namespace na {
+      template<typename T> T Y();
+      }
+      namespace na { void f() { nb::Y<int>(); } }
+      namespace nb { void g() { Y<int>(); } }
+      )";
+  std::string After = runClangRenameOnCode(Before, "na::X", "nb::Y");
+  CompareSnippets(Expected, After);
+}
+
 TEST_F(RenameFunctionTest, RenameOutOfLineFunctionDecls) {
   std::string Before = R"(
       namespace na {
         void X();
       }
       void na::X() {}
       )";
   std::string Expected = R"(