hokein updated this revision to Diff 118573.
hokein marked 3 inline comments as done.
hokein added a comment.
Address review comments.
https://reviews.llvm.org/D38723
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/RenameClassTest.cpp
Index: unittests/Rename/RenameClassTest.cpp
===================================================================
--- unittests/Rename/RenameClassTest.cpp
+++ unittests/Rename/RenameClassTest.cpp
@@ -469,8 +469,6 @@
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 @@
)";
std::string Expected = R"(
namespace ns {
- class ns::New {
+ class New {
public:
- ns::New() {}
+ New() {}
~New() {}
New* next() { return next_; }
@@ -504,8 +502,6 @@
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,32 +523,32 @@
)";
std::string Expected = R"(
namespace ns {
- class ns::New {
+ class New {
public:
- ns::New();
+ New();
~New();
New* next();
private:
New* next_;
};
- New::ns::New() {}
+ New::New() {}
New::~New() {}
New* New::next() { return next_; }
} // namespace ns
)";
std::string After = runClangRenameOnCode(Before, "ns::Old", "ns::New");
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 @@
})";
std::string Expected = R"(
namespace ns {
- class ns::New {
+ class New;
+ class New {
int x;
};
class Base {
@@ -615,7 +612,7 @@
)";
std::string Expected = R"(
namespace ns {
- class ::new_ns::New {
+ class New {
};
} // namespace ns
struct S {
@@ -632,7 +629,6 @@
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 @@
};
namespace ns {
- class ::new_ns::New {};
+ class New {};
void f() {
function<void(::new_ns::New)> func;
}
Index: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===================================================================
--- lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -160,13 +160,15 @@
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 +182,12 @@
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 +197,11 @@
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 +229,10 @@
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 +276,12 @@
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 +307,13 @@
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 +439,26 @@
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
