Author: hokein Date: Wed Oct 11 04:15:48 2017 New Revision: 315452 URL: http://llvm.org/viewvc/llvm-project?rev=315452&view=rev Log: [clang-rename] Don't add prefix qualifiers to the declaration and definition of the renamed symbol.
Reviewers: ioeric Reviewed By: ioeric Subscribers: klimek, cfe-commits, arphaman Differential Revision: https://reviews.llvm.org/D38723 Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp cfe/trunk/unittests/Rename/RenameClassTest.cpp Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp?rev=315452&r1=315451&r2=315452&view=diff ============================================================================== --- cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp (original) +++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp Wed Oct 11 04:15:48 2017 @@ -160,13 +160,14 @@ public: const Decl *Context; // The nested name being replaced (can be nullptr). const NestedNameSpecifier *Specifier; + // Determine whether the prefix qualifiers of the NewName should be ignored. + // Normally, we set it to true for the symbol declaration and definition to + // avoid adding prefix qualifiers. + // For example, if it is true and NewName is "a::b::foo", then the symbol + // occurrence which the RenameInfo points to will be renamed to "foo". + bool IgnorePrefixQualifers; }; - // FIXME: Currently, prefix qualifiers will be added to the renamed symbol - // definition (e.g. "class Foo {};" => "class b::Bar {};" when renaming - // "a::Foo" to "b::Bar"). - // For renaming declarations/definitions, prefix qualifiers should be filtered - // out. bool VisitNamedDecl(const NamedDecl *Decl) { // UsingDecl has been handled in other place. if (llvm::isa<UsingDecl>(Decl)) @@ -180,8 +181,12 @@ public: return true; if (isInUSRSet(Decl)) { - RenameInfo Info = {Decl->getLocation(), Decl->getLocation(), nullptr, - nullptr, nullptr}; + RenameInfo Info = {Decl->getLocation(), + Decl->getLocation(), + /*FromDecl=*/nullptr, + /*Context=*/nullptr, + /*Specifier=*/nullptr, + /*IgnorePrefixQualifers=*/true}; RenameInfos.push_back(Info); } return true; @@ -191,8 +196,11 @@ public: const NamedDecl *Decl = Expr->getFoundDecl(); if (isInUSRSet(Decl)) { RenameInfo Info = {Expr->getSourceRange().getBegin(), - Expr->getSourceRange().getEnd(), Decl, - getClosestAncestorDecl(*Expr), Expr->getQualifier()}; + Expr->getSourceRange().getEnd(), + Decl, + getClosestAncestorDecl(*Expr), + Expr->getQualifier(), + /*IgnorePrefixQualifers=*/false}; RenameInfos.push_back(Info); } @@ -220,8 +228,10 @@ public: if (isInUSRSet(TargetDecl)) { RenameInfo Info = {NestedLoc.getBeginLoc(), EndLocationForType(NestedLoc.getTypeLoc()), - TargetDecl, getClosestAncestorDecl(NestedLoc), - NestedLoc.getNestedNameSpecifier()->getPrefix()}; + TargetDecl, + getClosestAncestorDecl(NestedLoc), + NestedLoc.getNestedNameSpecifier()->getPrefix(), + /*IgnorePrefixQualifers=*/false}; RenameInfos.push_back(Info); } } @@ -265,9 +275,12 @@ public: if (!ParentTypeLoc.isNull() && isInUSRSet(getSupportedDeclFromTypeLoc(ParentTypeLoc))) return true; - RenameInfo Info = {StartLocationForType(Loc), EndLocationForType(Loc), - TargetDecl, getClosestAncestorDecl(Loc), - GetNestedNameForType(Loc)}; + RenameInfo Info = {StartLocationForType(Loc), + EndLocationForType(Loc), + TargetDecl, + getClosestAncestorDecl(Loc), + GetNestedNameForType(Loc), + /*IgnorePrefixQualifers=*/false}; RenameInfos.push_back(Info); return true; } @@ -293,11 +306,13 @@ public: llvm::isa<ElaboratedType>(ParentTypeLoc.getType())) TargetLoc = ParentTypeLoc; RenameInfo Info = { - StartLocationForType(TargetLoc), EndLocationForType(TargetLoc), + StartLocationForType(TargetLoc), + EndLocationForType(TargetLoc), TemplateSpecType->getTemplateName().getAsTemplateDecl(), getClosestAncestorDecl( ast_type_traits::DynTypedNode::create(TargetLoc)), - GetNestedNameForType(TargetLoc)}; + GetNestedNameForType(TargetLoc), + /*IgnorePrefixQualifers=*/false}; RenameInfos.push_back(Info); } } @@ -423,18 +438,26 @@ createRenameAtomicChanges(llvm::ArrayRef for (const auto &RenameInfo : Finder.getRenameInfos()) { std::string ReplacedName = NewName.str(); - if (RenameInfo.FromDecl && RenameInfo.Context) { - if (!llvm::isa<clang::TranslationUnitDecl>( - RenameInfo.Context->getDeclContext())) { - ReplacedName = tooling::replaceNestedName( - RenameInfo.Specifier, RenameInfo.Context->getDeclContext(), - RenameInfo.FromDecl, - NewName.startswith("::") ? NewName.str() : ("::" + NewName).str()); + if (RenameInfo.IgnorePrefixQualifers) { + // Get the name without prefix qualifiers from NewName. + size_t LastColonPos = NewName.find_last_of(':'); + if (LastColonPos != std::string::npos) + ReplacedName = NewName.substr(LastColonPos + 1); + } else { + if (RenameInfo.FromDecl && RenameInfo.Context) { + if (!llvm::isa<clang::TranslationUnitDecl>( + RenameInfo.Context->getDeclContext())) { + ReplacedName = tooling::replaceNestedName( + RenameInfo.Specifier, RenameInfo.Context->getDeclContext(), + RenameInfo.FromDecl, + NewName.startswith("::") ? NewName.str() + : ("::" + NewName).str()); + } } + // If the NewName contains leading "::", add it back. + if (NewName.startswith("::") && NewName.substr(2) == ReplacedName) + ReplacedName = NewName.str(); } - // If the NewName contains leading "::", add it back. - if (NewName.startswith("::") && NewName.substr(2) == ReplacedName) - ReplacedName = NewName.str(); Replace(RenameInfo.Begin, RenameInfo.End, ReplacedName); } Modified: cfe/trunk/unittests/Rename/RenameClassTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Rename/RenameClassTest.cpp?rev=315452&r1=315451&r2=315452&view=diff ============================================================================== --- cfe/trunk/unittests/Rename/RenameClassTest.cpp (original) +++ cfe/trunk/unittests/Rename/RenameClassTest.cpp Wed Oct 11 04:15:48 2017 @@ -469,8 +469,6 @@ TEST_F(ClangRenameTest, RenameClassWithI CompareSnippets(Expected, After); } -// FIXME: no prefix qualifiers being added to the class definition and -// constructor. TEST_F(ClangRenameTest, RenameClassWithNamespaceWithInlineMembers) { std::string Before = R"( namespace ns { @@ -488,9 +486,9 @@ TEST_F(ClangRenameTest, RenameClassWithN )"; std::string Expected = R"( namespace ns { - class ns::New { + class New { public: - ns::New() {} + New() {} ~New() {} New* next() { return next_; } @@ -504,8 +502,6 @@ TEST_F(ClangRenameTest, RenameClassWithN CompareSnippets(Expected, After); } -// FIXME: no prefix qualifiers being added to the class definition and -// constructor. TEST_F(ClangRenameTest, RenameClassWithNamespaceWithOutOfInlineMembers) { std::string Before = R"( namespace ns { @@ -527,9 +523,9 @@ TEST_F(ClangRenameTest, RenameClassWithN )"; std::string Expected = R"( namespace ns { - class ns::New { + class New { public: - ns::New(); + New(); ~New(); New* next(); @@ -538,7 +534,7 @@ TEST_F(ClangRenameTest, RenameClassWithN New* next_; }; - New::ns::New() {} + New::New() {} New::~New() {} New* New::next() { return next_; } } // namespace ns @@ -547,12 +543,12 @@ TEST_F(ClangRenameTest, RenameClassWithN CompareSnippets(Expected, After); } -// FIXME: no prefix qualifiers being added to the definition. TEST_F(ClangRenameTest, RenameClassInInheritedConstructor) { // `using Base::Base;` will generate an implicit constructor containing usage // of `::ns::Old` which should not be matched. std::string Before = R"( namespace ns { + class Old; class Old { int x; }; @@ -574,7 +570,8 @@ TEST_F(ClangRenameTest, RenameClassInInh })"; std::string Expected = R"( namespace ns { - class ns::New { + class New; + class New { int x; }; class Base { @@ -615,7 +612,7 @@ TEST_F(ClangRenameTest, DontRenameRefere )"; std::string Expected = R"( namespace ns { - class ::new_ns::New { + class New { }; } // namespace ns struct S { @@ -632,7 +629,6 @@ TEST_F(ClangRenameTest, DontRenameRefere CompareSnippets(Expected, After); } -// FIXME: no prefix qualifiers being adding to the definition. TEST_F(ClangRenameTest, ReferencesInLambdaFunctionParameters) { std::string Before = R"( template <class T> @@ -669,7 +665,7 @@ TEST_F(ClangRenameTest, ReferencesInLamb }; namespace ns { - class ::new_ns::New {}; + class New {}; void f() { function<void(::new_ns::New)> func; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
