ioeric created this revision.
ioeric added a reviewer: hokein.
Herald added subscribers: cfe-commits, klimek.
Previously, the matcher matches a function call/ref multiple times, one
for each decl ancestor. This might cause problems. For example, in the following
case, `func()` would be matched once (with namespace context) before using decl
is
seen and once after using decl is seeing, which would result in different
conflicting
replacements as the first match would replace `func` with "ns::func" as it
doesn't
know about the using decl.
namespace x {
namespace {
using ::ns::func;
void f() { func(); }
}
}
Switching from `hasDescendant` matching to `hasAncestor` matching solves the
problem.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44517
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===================================================================
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
std::string Code = "namespace glob {\n"
"class Glob {};\n"
+ "void GFunc() {}\n"
"}\n"
"using glob::Glob;\n"
+ "using glob::GFunc;\n"
"namespace na {\n"
"namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
"} // namespace nb\n"
"} // namespace na\n";
std::string Expected = "namespace glob {\n"
"class Glob {};\n"
+ "void GFunc() {}\n"
"}\n"
"using glob::Glob;\n"
+ "using glob::GFunc;\n"
"\n"
"namespace x {\n"
"namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+ std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "} // namespace util\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
"} // namespace y\n"
"} // namespace x\n";
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
Index: change-namespace/ChangeNamespace.cpp
===================================================================
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
hasAncestor(namespaceDecl(isAnonymous())),
hasAncestor(cxxRecordDecl()))),
hasParent(namespaceDecl()));
- Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
- callExpr(callee(FuncMatcher)).bind("call"),
- declRefExpr(to(FuncMatcher.bind("func_decl")))
- .bind("func_ref")))),
- IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+ Finder->addMatcher(
+ expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+ declRefExpr(to(FuncMatcher.bind("func_decl")))
+ .bind("func_ref")))),
+ this);
auto GlobalVarMatcher = varDecl(
hasGlobalStorage(), hasParent(namespaceDecl()),
@@ -509,6 +509,7 @@
void ChangeNamespaceTool::run(
const ast_matchers::MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
+ << Using->getQualifiedNameAsString() << "\n";
UsingDecls.insert(Using);
} else if (const auto *UsingNamespace =
Result.Nodes.getNodeAs<UsingDirectiveDecl>(
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===================================================================
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
std::string Code = "namespace glob {\n"
"class Glob {};\n"
+ "void GFunc() {}\n"
"}\n"
"using glob::Glob;\n"
+ "using glob::GFunc;\n"
"namespace na {\n"
"namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
"} // namespace nb\n"
"} // namespace na\n";
std::string Expected = "namespace glob {\n"
"class Glob {};\n"
+ "void GFunc() {}\n"
"}\n"
"using glob::Glob;\n"
+ "using glob::GFunc;\n"
"\n"
"namespace x {\n"
"namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+ std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "} // namespace util\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
"} // namespace y\n"
"} // namespace x\n";
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
Index: change-namespace/ChangeNamespace.cpp
===================================================================
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
hasAncestor(namespaceDecl(isAnonymous())),
hasAncestor(cxxRecordDecl()))),
hasParent(namespaceDecl()));
- Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
- callExpr(callee(FuncMatcher)).bind("call"),
- declRefExpr(to(FuncMatcher.bind("func_decl")))
- .bind("func_ref")))),
- IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+ Finder->addMatcher(
+ expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+ declRefExpr(to(FuncMatcher.bind("func_decl")))
+ .bind("func_ref")))),
+ this);
auto GlobalVarMatcher = varDecl(
hasGlobalStorage(), hasParent(namespaceDecl()),
@@ -509,6 +509,7 @@
void ChangeNamespaceTool::run(
const ast_matchers::MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
+ << Using->getQualifiedNameAsString() << "\n";
UsingDecls.insert(Using);
} else if (const auto *UsingNamespace =
Result.Nodes.getNodeAs<UsingDirectiveDecl>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
