Author: ioeric
Date: Thu Mar 15 07:45:02 2018
New Revision: 327629
URL: http://llvm.org/viewvc/llvm-project?rev=327629&view=rev
Log:
[change-namespace] Don't match a function call/ref multiple times.
Summary:
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.
Reviewers: hokein
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D44517
Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=327629&r1=327628&r2=327629&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Thu Mar 15
07:45:02 2018
@@ -478,13 +478,13 @@ void ChangeNamespaceTool::registerMatche
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()),
Modified:
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=327629&r1=327628&r2=327629&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
(original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
Thu Mar 15 07:45:02 2018
@@ -850,22 +850,58 @@ TEST_F(ChangeNamespaceTest, CommentsBefo
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));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
