ioeric updated this revision to Diff 72275.
ioeric added a comment.
- Update comment.
https://reviews.llvm.org/D24862
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/tool/ClangChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===================================================================
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -229,8 +229,23 @@
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
}
+TEST_F(ChangeNamespaceTest, DoNotCrashWithLambdaAsParameter) {
+ std::string Code =
+ "#include <functional>\n"
+ "void f(std::function<void(int)> func, int param) { func(param); } "
+ "void g() { f([](int x) {}, 1); }";
+
+ std::string Expected =
+ "#include <functional>\n"
+ "void f(std::function<void(int)> func, int param) { func(param); } "
+ "void g() { f([](int x) {}, 1); }";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
TEST_F(ChangeNamespaceTest, FixUsingShadowDecl) {
- std::string Code = "namespace na {\n"
+ std::string Code = "class GLOB {};\n"
+ "using BLOG = GLOB;\n"
+ "namespace na {\n"
"namespace nc {\n"
"class SAME {};\n"
"}\n"
@@ -245,7 +260,9 @@
"} // namespace nb\n"
"} // namespace na\n";
- std::string Expected = "namespace na {\n"
+ std::string Expected = "class GLOB {};\n"
+ "using BLOG = GLOB;\n"
+ "namespace na {\n"
"namespace nc {\n"
"class SAME {};\n"
"}\n"
@@ -265,6 +282,85 @@
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
}
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInFunction) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() {\n"
+ " using glob::Glob;\n"
+ " Glob g;\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ // FIXME: don't add namespace qualifier when there is UsingShadowDecl.
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() {\n"
+ " using ::glob::Glob;\n"
+ " glob::Glob g;\n"
+ "}\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using glob::Glob;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ // FIXME: don't add namespace qualifier when there is UsingShadowDecl.
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using glob::Glob;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingNamespace) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using namespace glob;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ // FIXME: don't add namespace qualifier when there is "using namespace" decl.
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using namespace glob;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) {
std::string Code =
"namespace na {\n"
Index: change-namespace/tool/ClangChangeNamespace.cpp
===================================================================
--- change-namespace/tool/ClangChangeNamespace.cpp
+++ change-namespace/tool/ClangChangeNamespace.cpp
@@ -38,6 +38,7 @@
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
using namespace clang;
using namespace llvm;
@@ -69,6 +70,7 @@
} // anonymous namespace
int main(int argc, const char **argv) {
+ sys::PrintStackTraceOnErrorSignal(argv[0]);
tooling::CommonOptionsParser OptionsParser(argc, argv,
ChangeNamespaceCategory);
const auto &Files = OptionsParser.getSourcePathList();
Index: change-namespace/ChangeNamespace.cpp
===================================================================
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -256,9 +256,10 @@
auto DeclMatcher = namedDecl(
hasAncestor(namespaceDecl()),
unless(anyOf(
- hasAncestor(namespaceDecl(isAnonymous())),
+ isImplicit(), hasAncestor(namespaceDecl(isAnonymous())),
hasAncestor(cxxRecordDecl()),
allOf(IsInMovedNs, unless(cxxRecordDecl(unless(isDefinition())))))));
+
// Match TypeLocs on the declaration. Carefully match only the outermost
// TypeLoc that's directly linked to the old class and don't handle nested
// name specifier locs.
@@ -271,10 +272,13 @@
hasAncestor(decl().bind("dc")))
.bind("type"),
this);
+
// Types in `UsingShadowDecl` is not matched by `typeLoc` above, so we need to
// special case it.
Finder->addMatcher(
- usingDecl(hasAnyUsingShadowDecl(IsInMovedNs)).bind("using_decl"), this);
+ usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl())).bind("using_decl"),
+ this);
+
// Handle types in nested name specifier.
Finder->addMatcher(nestedNameSpecifierLoc(
hasAncestor(decl(IsInMovedNs).bind("dc")),
@@ -284,18 +288,23 @@
this);
// Handle function.
- // Only handle functions that are defined in a namespace excluding static
- // methods (qualified by nested specifier) and functions defined in the global
- // namespace.
+ // Only handle functions that are defined in a namespace excluding member
+ // function, static methods (qualified by nested specifier), and functions
+ // defined in the global namespace.
// Note that the matcher does not exclude calls to out-of-line static method
// definitions, so we need to exclude them in the callback handler.
- auto FuncMatcher = functionDecl(
- hasParent(namespaceDecl()),
- unless(anyOf(IsInMovedNs, hasAncestor(namespaceDecl(isAnonymous())),
- hasAncestor(cxxRecordDecl()))));
+ // NOTE: ASTMatcher crash when `FunctionDecl` is a lambda defined in parameter
+ // list, in which case it has no parent map. Workaround by filtering out
+ // `CxxMethodDecl` (which matches lambda, and we do want to match
+ // `CXXMethodDecl` after all).
+ auto FuncMatcher =
+ functionDecl(unless(anyOf(cxxMethodDecl(), IsInMovedNs,
+ hasAncestor(namespaceDecl(isAnonymous())),
+ hasAncestor(cxxRecordDecl()))),
+ hasParent(namespaceDecl()));
Finder->addMatcher(
decl(forEachDescendant(callExpr(callee(FuncMatcher)).bind("call")),
- IsInMovedNs)
+ IsInMovedNs, unless(isImplicit()))
.bind("dc"),
this);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
