Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp =================================================================== --- cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp +++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp @@ -39,6 +39,17 @@ namespace { +// Returns true if the given Loc is valid for edit. We don't edit the +// SourceLocations that are valid or in temporary buffer. +bool IsValidEditLoc(const clang::SourceManager& SM, clang::SourceLocation Loc) { + if (Loc.isInvalid()) + return false; + const clang::FullSourceLoc FullLoc(Loc, SM); + std::pair FileIdAndOffset = + FullLoc.getSpellingLoc().getDecomposedLoc(); + return SM.getFileEntryForID(FileIdAndOffset.first) != nullptr; +} + // \brief This visitor recursively searches for all instances of a USR in a // translation unit and stores them for later usage. class USRLocFindingASTVisitor @@ -181,13 +192,22 @@ return true; if (isInUSRSet(Decl)) { - RenameInfo Info = {Decl->getLocation(), - Decl->getLocation(), - /*FromDecl=*/nullptr, - /*Context=*/nullptr, - /*Specifier=*/nullptr, - /*IgnorePrefixQualifers=*/true}; - RenameInfos.push_back(Info); + // For the case of renaming an alias template, we actually rename the + // underlying alias declaration of the template. + if (const auto* TAT = dyn_cast(Decl)) + Decl = TAT->getTemplatedDecl(); + + auto StartLoc = Decl->getLocation(); + auto EndLoc = StartLoc; + if (IsValidEditLoc(Context.getSourceManager(), StartLoc)) { + RenameInfo Info = {StartLoc, + EndLoc, + /*FromDecl=*/nullptr, + /*Context=*/nullptr, + /*Specifier=*/nullptr, + /*IgnorePrefixQualifers=*/true}; + RenameInfos.push_back(Info); + } } return true; } @@ -200,7 +220,7 @@ Decl = UsingShadow->getTargetDecl(); } - auto BeginLoc = Expr->getLocStart(); + auto StartLoc = Expr->getLocStart(); auto EndLoc = Expr->getLocEnd(); // In case of renaming an enum declaration, we have to explicitly handle // unscoped enum constants referenced in expressions (e.g. @@ -233,8 +253,9 @@ assert(EndLoc.isValid() && "The enum constant should have prefix qualifers."); } - if (isInUSRSet(Decl)) { - RenameInfo Info = {BeginLoc, + if (isInUSRSet(Decl) && + IsValidEditLoc(Context.getSourceManager(), StartLoc)) { + RenameInfo Info = {StartLoc, EndLoc, Decl, getClosestAncestorDecl(*Expr), @@ -259,8 +280,6 @@ bool VisitNestedNameSpecifierLocations(NestedNameSpecifierLoc NestedLoc) { if (!NestedLoc.getNestedNameSpecifier()->getAsType()) return true; - if (IsTypeAliasWhichWillBeRenamedElsewhere(NestedLoc.getTypeLoc())) - return true; if (const auto *TargetDecl = getSupportedDeclFromTypeLoc(NestedLoc.getTypeLoc())) { @@ -278,9 +297,6 @@ } bool VisitTypeLoc(TypeLoc Loc) { - if (IsTypeAliasWhichWillBeRenamedElsewhere(Loc)) - return true; - auto Parents = Context.getParents(Loc); TypeLoc ParentTypeLoc; if (!Parents.empty()) { @@ -314,13 +330,18 @@ if (!ParentTypeLoc.isNull() && isInUSRSet(getSupportedDeclFromTypeLoc(ParentTypeLoc))) return true; - RenameInfo Info = {StartLocationForType(Loc), - EndLocationForType(Loc), - TargetDecl, - getClosestAncestorDecl(Loc), - GetNestedNameForType(Loc), - /*IgnorePrefixQualifers=*/false}; - RenameInfos.push_back(Info); + + auto StartLoc = StartLocationForType(Loc); + auto EndLoc = EndLocationForType(Loc); + if (IsValidEditLoc(Context.getSourceManager(), StartLoc)) { + RenameInfo Info = {StartLoc, + EndLoc, + TargetDecl, + getClosestAncestorDecl(Loc), + GetNestedNameForType(Loc), + /*IgnorePrefixQualifers=*/false}; + RenameInfos.push_back(Info); + } return true; } } @@ -344,15 +365,20 @@ if (!ParentTypeLoc.isNull() && llvm::isa(ParentTypeLoc.getType())) TargetLoc = ParentTypeLoc; - RenameInfo Info = { - StartLocationForType(TargetLoc), - EndLocationForType(TargetLoc), - TemplateSpecType->getTemplateName().getAsTemplateDecl(), - getClosestAncestorDecl( - ast_type_traits::DynTypedNode::create(TargetLoc)), - GetNestedNameForType(TargetLoc), - /*IgnorePrefixQualifers=*/false}; - RenameInfos.push_back(Info); + + auto StartLoc = StartLocationForType(TargetLoc); + auto EndLoc = EndLocationForType(TargetLoc); + if (IsValidEditLoc(Context.getSourceManager(), StartLoc)) { + RenameInfo Info = { + StartLoc, + EndLoc, + TemplateSpecType->getTemplateName().getAsTemplateDecl(), + getClosestAncestorDecl( + ast_type_traits::DynTypedNode::create(TargetLoc)), + GetNestedNameForType(TargetLoc), + /*IgnorePrefixQualifers=*/false}; + RenameInfos.push_back(Info); + } } } return true; @@ -367,38 +393,11 @@ } private: - // FIXME: This method may not be suitable for renaming other types like alias - // types. Need to figure out a way to handle it. - bool IsTypeAliasWhichWillBeRenamedElsewhere(TypeLoc TL) const { - while (!TL.isNull()) { - // SubstTemplateTypeParm is the TypeLocation class for a substituted type - // inside a template expansion so we ignore these. For example: - // - // template struct S { - // T t; // <-- this T becomes a TypeLoc(int) with class - // // SubstTemplateTypeParm when S is instantiated - // } - if (TL.getTypeLocClass() == TypeLoc::SubstTemplateTypeParm) - return true; - - // Typedef is the TypeLocation class for a type which is a typedef to the - // type we want to replace. We ignore the use of the typedef as we will - // replace the definition of it. For example: - // - // typedef int T; - // T a; // <--- This T is a TypeLoc(int) with class Typedef. - if (TL.getTypeLocClass() == TypeLoc::Typedef) - return true; - TL = TL.getNextTypeLoc(); - } - return false; - } - // Get the supported declaration from a given typeLoc. If the declaration type // is not supported, returns nullptr. - // - // FIXME: support more types, e.g. type alias. const NamedDecl *getSupportedDeclFromTypeLoc(TypeLoc Loc) { + if (const auto* TT = Loc.getType()->getAs()) + return TT->getDecl(); if (const auto *RD = Loc.getType()->getAsCXXRecordDecl()) return RD; if (const auto *ED = Index: cfe/trunk/unittests/Rename/CMakeLists.txt =================================================================== --- cfe/trunk/unittests/Rename/CMakeLists.txt +++ cfe/trunk/unittests/Rename/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_unittest(ClangRenameTests RenameClassTest.cpp RenameEnumTest.cpp + RenameAliasTest.cpp RenameFunctionTest.cpp ) Index: cfe/trunk/unittests/Rename/RenameAliasTest.cpp =================================================================== --- cfe/trunk/unittests/Rename/RenameAliasTest.cpp +++ cfe/trunk/unittests/Rename/RenameAliasTest.cpp @@ -0,0 +1,304 @@ +//===-- RenameAliasTest.cpp - unit tests for renaming alias ---------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "ClangRenameTest.h" + +namespace clang { +namespace clang_rename { +namespace test { +namespace { + +class RenameAliasTest : public ClangRenameTest { +public: + RenameAliasTest() { + AppendToHeader(R"( + #define MACRO(x) x + namespace some_ns { + class A { + public: + void foo() {} + struct Nested { + enum NestedEnum { + E1, E2, + }; + }; + }; + } // namespace some_ns + namespace a { + typedef some_ns::A TA; + using UA = some_ns::A; + } // namespace a + namespace b { + typedef some_ns::A TA; + using UA = some_ns::A; + } + template class ptr {}; + template + + using TPtr = ptr; + )"); + } +}; + +INSTANTIATE_TEST_CASE_P( + RenameAliasTests, RenameAliasTest, + testing::ValuesIn(std::vector({ + // basic functions + {"void f(a::TA a1) {}", "void f(b::TB a1) {}", "a::TA", "b::TB"}, + {"void f(a::UA a1) {}", "void f(b::UB a1) {}", "a::UA", "b::UB"}, + {"void f(a::TA* a1) {}", "void f(b::TB* a1) {}", "a::TA", "b::TB"}, + {"void f(a::TA** a1) {}", "void f(b::TB** a1) {}", "a::TA", "b::TB"}, + {"a::TA f() { return a::TA(); }", "b::TB f() { return b::TB(); }", + "a::TA", "b::TB"}, + {"a::TA f() { return a::UA(); }", "b::TB f() { return a::UA(); }", + "a::TA", "b::TB"}, + {"a::TA f() { return a::UA(); }", "a::TA f() { return b::UB(); }", + "a::UA", "b::UB"}, + {"void f() { a::TA a; }", "void f() { b::TB a; }", "a::TA", "b::TB"}, + {"void f(const a::TA& a1) {}", "void f(const b::TB& a1) {}", "a::TA", + "b::TB"}, + {"void f(const a::UA& a1) {}", "void f(const b::UB& a1) {}", "a::UA", + "b::UB"}, + {"void f(const a::TA* a1) {}", "void f(const b::TB* a1) {}", "a::TA", + "b::TB"}, + {"namespace a { void f(TA a1) {} }", + "namespace a { void f(b::TB a1) {} }", "a::TA", "b::TB"}, + {"void f(MACRO(a::TA) a1) {}", "void f(MACRO(b::TB) a1) {}", "a::TA", + "b::TB"}, + {"void f(MACRO(a::TA a1)) {}", "void f(MACRO(b::TB a1)) {}", "a::TA", + "b::TB"}, + + // shorten/add namespace. + {"namespace b { void f(a::UA a1) {} }", + "namespace b {void f(UB a1) {} }", "a::UA", "b::UB"}, + {"namespace a { void f(UA a1) {} }", + "namespace a {void f(b::UB a1) {} }", "a::UA", "b::UB"}, + + // use namespace and typedefs + {"struct S { using T = a::TA; T a_; };", + "struct S { using T = b::TB; T a_; };", "a::TA", "b::TB"}, + {"using T = a::TA; T gA;", "using T = b::TB; T gA;", "a::TA", "b::TB"}, + {"using T = a::UA; T gA;", "using T = b::UB; T gA;", "a::UA", "b::UB"}, + {"typedef a::TA T; T gA;", "typedef b::TB T; T gA;", "a::TA", "b::TB"}, + {"typedef a::UA T; T gA;", "typedef b::UB T; T gA;", "a::UA", "b::UB"}, + {"typedef MACRO(a::TA) T; T gA;", "typedef MACRO(b::TB) T; T gA;", + "a::TA", "b::TB"}, + + // types in using shadows. + {"using a::TA; TA gA;", "using b::TB; b::TB gA;", "a::TA", "b::TB"}, + {"using a::UA; UA gA;", "using b::UB; b::UB gA;", "a::UA", "b::UB"}, + + // struct members and other oddities + {"struct S : public a::TA {};", "struct S : public b::TB {};", "a::TA", + "b::TB"}, + {"struct S : public a::UA {};", "struct S : public b::UB {};", "a::UA", + "b::UB"}, + {"struct F { void f(a::TA a1) {} };", + "struct F { void f(b::TB a1) {} };", "a::TA", "b::TB"}, + {"struct F { a::TA a_; };", "struct F { b::TB a_; };", "a::TA", + "b::TB"}, + {"struct F { ptr a_; };", "struct F { ptr a_; };", + "a::TA", "b::TB"}, + {"struct F { ptr a_; };", "struct F { ptr a_; };", + "a::UA", "b::UB"}, + + // types in nested name specifiers + {"void f() { a::TA::Nested ne; }", "void f() { b::TB::Nested ne; }", + "a::TA", "b::TB"}, + {"void f() { a::UA::Nested ne; }", "void f() { b::UB::Nested ne; }", + "a::UA", "b::UB"}, + {"void f() { a::TA::Nested::NestedEnum e; }", + "void f() { b::TB::Nested::NestedEnum e; }", "a::TA", "b::TB"}, + {"void f() { auto e = a::TA::Nested::NestedEnum::E1; }", + "void f() { auto e = b::TB::Nested::NestedEnum::E1; }", "a::TA", + "b::TB"}, + {"void f() { auto e = a::TA::Nested::E1; }", + "void f() { auto e = b::TB::Nested::E1; }", "a::TA", "b::TB"}, + + // templates + {"template struct Foo { T t; }; void f() { Foo " + "foo; }", + "template struct Foo { T t; }; void f() { Foo " + "foo; }", + "a::TA", "b::TB"}, + {"template struct Foo { a::TA a; };", + "template struct Foo { b::TB a; };", "a::TA", "b::TB"}, + {"template void f(T t) {} void g() { f(a::TA()); }", + "template void f(T t) {} void g() { f(b::TB()); }", + "a::TA", "b::TB"}, + {"template void f(T t) {} void g() { f(a::UA()); }", + "template void f(T t) {} void g() { f(b::UB()); }", + "a::UA", "b::UB"}, + {"template int f() { return 1; } template <> int " + "f() { return 2; } int g() { return f(); }", + "template int f() { return 1; } template <> int " + "f() { return 2; } int g() { return f(); }", + "a::TA", "b::TB"}, + {"struct Foo { template T foo(); }; void g() { Foo f; " + "auto a = f.template foo(); }", + "struct Foo { template T foo(); }; void g() { Foo f; " + "auto a = f.template foo(); }", + "a::TA", "b::TB"}, + {"struct Foo { template T foo(); }; void g() { Foo f; " + "auto a = f.template foo(); }", + "struct Foo { template T foo(); }; void g() { Foo f; " + "auto a = f.template foo(); }", + "a::UA", "b::UB"}, + + // The following two templates are distilled from regressions found in + // unique_ptr<> and type_traits.h + {"template struct outer { typedef T type; type Baz(); }; " + "outer g_A;", + "template struct outer { typedef T type; type Baz(); }; " + "outer g_A;", + "a::TA", "b::TB"}, + {"template struct nested { typedef T type; }; template " + " struct outer { typename nested::type Foo(); }; " + "outer g_A;", + "template struct nested { typedef T type; }; template " + " struct outer { typename nested::type Foo(); }; " + "outer g_A;", + "a::TA", "b::TB"}, + + // macros + {"#define FOO(T, t) T t\nvoid f() { FOO(a::TA, a1); FOO(a::TA, a2); }", + "#define FOO(T, t) T t\nvoid f() { FOO(b::TB, a1); FOO(b::TB, a2); }", + "a::TA", "b::TB"}, + {"#define FOO(n) a::TA n\nvoid f() { FOO(a1); FOO(a2); }", + "#define FOO(n) b::TB n\nvoid f() { FOO(a1); FOO(a2); }", "a::TA", + "b::TB"}, + {"#define FOO(n) a::UA n\nvoid f() { FOO(a1); FOO(a2); }", + "#define FOO(n) b::UB n\nvoid f() { FOO(a1); FOO(a2); }", "a::UA", + "b::UB"}, + + // Pointer to member functions + {"auto gA = &a::TA::foo;", "auto gA = &b::TB::foo;", "a::TA", "b::TB"}, + {"using a::TA; auto gA = &TA::foo;", + "using b::TB; auto gA = &b::TB::foo;", "a::TA", "b::TB"}, + {"typedef a::TA T; auto gA = &T::foo;", + "typedef b::TB T; auto gA = &T::foo;", "a::TA", "b::TB"}, + {"auto gA = &MACRO(a::TA)::foo;", "auto gA = &MACRO(b::TB)::foo;", + "a::TA", "b::TB"}, + + // templated using alias. + {"void f(TPtr p) {}", "void f(NewTPtr p) {}", "TPtr", + "NewTPtr"}, + {"void f(::TPtr p) {}", "void f(::NewTPtr p) {}", "TPtr", + "NewTPtr"}, + })), ); + +TEST_P(RenameAliasTest, RenameAlias) { + auto Param = GetParam(); + assert(!Param.OldName.empty()); + assert(!Param.NewName.empty()); + std::string Actual = + runClangRenameOnCode(Param.Before, Param.OldName, Param.NewName); + CompareSnippets(Param.After, Actual); +} + +TEST_F(RenameAliasTest, RenameTypedefDefinitions) { + std::string Before = R"( + class X {}; + typedef X TOld; + )"; + std::string Expected = R"( + class X {}; + typedef X TNew; + )"; + std::string After = runClangRenameOnCode(Before, "TOld", "TNew"); + CompareSnippets(Expected, After); +} + +TEST_F(RenameAliasTest, RenameUsingAliasDefinitions) { + std::string Before = R"( + class X {}; + using UOld = X; + )"; + std::string Expected = R"( + class X {}; + using UNew = X; + )"; + std::string After = runClangRenameOnCode(Before, "UOld", "UNew"); + CompareSnippets(Expected, After); +} + +TEST_F(RenameAliasTest, RenameTemplatedAliasDefinitions) { + std::string Before = R"( + template + class X { T t; }; + + template + using Old = X; + )"; + std::string Expected = R"( + template + class X { T t; }; + + template + using New = X; + )"; + std::string After = runClangRenameOnCode(Before, "Old", "New"); + CompareSnippets(Expected, After); +} + +TEST_F(RenameAliasTest, RenameAliasesInNamespaces) { + std::string Before = R"( + namespace x { class X {}; } + namespace ns { + using UOld = x::X; + } + )"; + std::string Expected = R"( + namespace x { class X {}; } + namespace ns { + using UNew = x::X; + } + )"; + std::string After = runClangRenameOnCode(Before, "ns::UOld", "ns::UNew"); + CompareSnippets(Expected, After); +} + +TEST_F(RenameAliasTest, AliasesInMacros) { + std::string Before = R"( + namespace x { class Old {}; } + namespace ns { + #define REF(alias) alias alias_var; + + #define ALIAS(old) \ + using old##Alias = x::old; \ + REF(old##Alias); + + ALIAS(Old); + + OldAlias old_alias; + } + )"; + std::string Expected = R"( + namespace x { class Old {}; } + namespace ns { + #define REF(alias) alias alias_var; + + #define ALIAS(old) \ + using old##Alias = x::old; \ + REF(old##Alias); + + ALIAS(Old); + + NewAlias old_alias; + } + )"; + std::string After = + runClangRenameOnCode(Before, "ns::OldAlias", "ns::NewAlias"); + CompareSnippets(Expected, After); +} + +} // anonymous namespace +} // namespace test +} // namespace clang_rename +} // namesdpace clang