[clang-tools-extra] r287118 - [change-namespace] handle constructor initializer: Derived : Base::Base() {} and added conflict detections
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 16 08:54:54 PST 2016
Author: ioeric
Date: Wed Nov 16 10:54:53 2016
New Revision: 287118
URL: http://llvm.org/viewvc/llvm-project?rev=287118&view=rev
Log:
[change-namespace] handle constructor initializer: Derived : Base::Base() {} and added conflict detections
Summary:
namespace nx { namespace ny { class Base { public: Base(i) {}} } }
namespace na {
namespace nb {
class X : public nx::ny {
public:
X() : Base::Base(1) {}
};
}
}
When changing from na::nb to x::y, "Base::Base" will be changed to "nx::ny::Base" and
"Base::" in "Base::Base" will be replaced with "nx::ny::Base" too, which causes
conflict. This conflict should've been detected when adding replacements but was hidden by `addOrMergeReplacement`. We now also detect conflict when adding replacements where conflict must not happen.
The namespace lookup is tricky here, we simply replace "Base::Base()" with "nx::ny::Base()" as a workaround, which compiles but not perfect.
Reviewers: hokein
Subscribers: bkramer, cfe-commits
Differential Revision: https://reviews.llvm.org/D26637
Added:
clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp
Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
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=287118&r1=287117&r2=287118&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Wed Nov 16 10:54:53 2016
@@ -9,6 +9,7 @@
#include "ChangeNamespace.h"
#include "clang/Format/Format.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace clang::ast_matchers;
@@ -336,14 +337,27 @@ void ChangeNamespaceTool::registerMatche
.bind("using_with_shadow"),
this);
- // Handle types in nested name specifier.
+ // Handle types in nested name specifier. Specifiers that are in a TypeLoc
+ // matched above are not matched, e.g. "A::" in "A::A" is not matched since
+ // "A::A" would have already been fixed.
Finder->addMatcher(nestedNameSpecifierLoc(
hasAncestor(decl(IsInMovedNs).bind("dc")),
loc(nestedNameSpecifier(specifiesType(
- hasDeclaration(DeclMatcher.bind("from_decl"))))))
+ hasDeclaration(DeclMatcher.bind("from_decl"))))),
+ unless(hasAncestor(typeLoc(loc(qualType(hasDeclaration(
+ decl(equalsBoundNode("from_decl")))))))))
.bind("nested_specifier_loc"),
this);
+ // Matches base class initializers in constructors. TypeLocs of base class
+ // initializers do not need to be fixed. For example,
+ // class X : public a::b::Y {
+ // public:
+ // X() : Y::Y() {} // Y::Y do not need namespace specifier.
+ // };
+ Finder->addMatcher(
+ cxxCtorInitializer(isBaseInitializer()).bind("base_initializer"), this);
+
// Handle function.
// Only handle functions that are defined in a namespace excluding member
// function, static methods (qualified by nested specifier), and functions
@@ -393,6 +407,11 @@ void ChangeNamespaceTool::run(
SourceLocation Start = Specifier->getBeginLoc();
SourceLocation End = EndLocationForType(Specifier->getTypeLoc());
fixTypeLoc(Result, Start, End, Specifier->getTypeLoc());
+ } else if (const auto *BaseInitializer =
+ Result.Nodes.getNodeAs<CXXCtorInitializer>(
+ "base_initializer")) {
+ BaseCtorInitializerTypeLocs.push_back(
+ BaseInitializer->getTypeSourceInfo()->getTypeLoc());
} else if (const auto *TLoc = Result.Nodes.getNodeAs<TypeLoc>("type")) {
fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc),
*TLoc);
@@ -520,7 +539,9 @@ void ChangeNamespaceTool::moveClassForwa
// Delete the forward declaration from the code to be moved.
const auto Deletion =
createReplacement(Start, End, "", *Result.SourceManager);
- addOrMergeReplacement(Deletion, &FileToReplacements[Deletion.getFilePath()]);
+ auto Err = FileToReplacements[Deletion.getFilePath()].add(Deletion);
+ if (Err)
+ llvm_unreachable(llvm::toString(std::move(Err)).c_str());
llvm::StringRef Code = Lexer::getSourceText(
CharSourceRange::getTokenRange(
Result.SourceManager->getSpellingLoc(Start),
@@ -606,7 +627,9 @@ void ChangeNamespaceTool::replaceQualifi
if (NestedName == ReplaceName)
return;
auto R = createReplacement(Start, End, ReplaceName, *Result.SourceManager);
- addOrMergeReplacement(R, &FileToReplacements[R.getFilePath()]);
+ auto Err = FileToReplacements[R.getFilePath()].add(R);
+ if (Err)
+ llvm_unreachable(llvm::toString(std::move(Err)).c_str());
}
// Replace the [Start, End] of `Type` with the shortest qualified name when the
@@ -617,6 +640,9 @@ void ChangeNamespaceTool::fixTypeLoc(
// FIXME: do not rename template parameter.
if (Start.isInvalid() || End.isInvalid())
return;
+ // Types of CXXCtorInitializers do not need to be fixed.
+ if (llvm::is_contained(BaseCtorInitializerTypeLocs, Type))
+ return;
// The declaration which this TypeLoc refers to.
const auto *FromDecl = Result.Nodes.getNodeAs<NamedDecl>("from_decl");
// `hasDeclaration` gives underlying declaration, but if the type is
@@ -667,7 +693,9 @@ void ChangeNamespaceTool::fixUsingShadow
// Use fully qualified name in UsingDecl for now.
auto R = createReplacement(Start, End, "using ::" + TargetDeclName,
*Result.SourceManager);
- addOrMergeReplacement(R, &FileToReplacements[R.getFilePath()]);
+ auto Err = FileToReplacements[R.getFilePath()].add(R);
+ if (Err)
+ llvm_unreachable(llvm::toString(std::move(Err)).c_str());
}
void ChangeNamespaceTool::onEndOfTranslationUnit() {
Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.h?rev=287118&r1=287117&r2=287118&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.h (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.h Wed Nov 16 10:54:53 2016
@@ -145,6 +145,9 @@ private:
// Records all using namespace declarations, which can be used to shorten
// namespace specifiers.
llvm::SmallPtrSet<const UsingDirectiveDecl *, 8> UsingNamespaceDecls;
+ // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need to
+ // be fixed.
+ llvm::SmallVector<TypeLoc, 8> BaseCtorInitializerTypeLocs;
};
} // namespace change_namespace
Added: clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp?rev=287118&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp (added)
+++ clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp Wed Nov 16 10:54:53 2016
@@ -0,0 +1,22 @@
+// RUN: clang-change-namespace -old_namespace "na::nb" -new_namespace "x::y" --file_pattern ".*" %s -- -std=c++11 | sed 's,// CHECK.*,,' | FileCheck %s
+
+template <class T>
+class function;
+template <class R, class... ArgTypes>
+class function<R(ArgTypes...)> {
+public:
+ template <typename Functor>
+ function(Functor f) {}
+ R operator()(ArgTypes...) const {}
+};
+
+// CHECK: namespace x {
+// CHECK-NEXT: namespace y {
+namespace na {
+namespace nb {
+void f(function<void(int)> func, int param) { func(param); }
+void g() { f([](int x) {}, 1); }
+// CHECK: } // namespace y
+// CHECK-NEXT: } // namespace x
+}
+}
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=287118&r1=287117&r2=287118&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp Wed Nov 16 10:54:53 2016
@@ -43,8 +43,9 @@ public:
NamespaceTool.registerMatchers(&Finder);
std::unique_ptr<tooling::FrontendActionFactory> Factory =
tooling::newFrontendActionFactory(&Finder);
- tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"},
- FileName);
+ if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"},
+ FileName))
+ return "";
formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite);
return format(Context.getRewrittenText(ID));
}
@@ -330,19 +331,6 @@ TEST_F(ChangeNamespaceTest, MoveFunction
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 = "class GLOB {};\n"
"using BLOG = GLOB;\n"
@@ -510,8 +498,8 @@ TEST_F(ChangeNamespaceTest, DoNotFixStat
"public:\n"
"static int A1;\n"
"static int A2;\n"
- "}\n"
- "static int A::A1 = 0;\n"
+ "};\n"
+ "int A::A1 = 0;\n"
"namespace nb {\n"
"void f() { int a = A::A1; int b = A::A2; }"
"} // namespace nb\n"
@@ -522,8 +510,8 @@ TEST_F(ChangeNamespaceTest, DoNotFixStat
"public:\n"
"static int A1;\n"
"static int A2;\n"
- "}\n"
- "static int A::A1 = 0;\n"
+ "};\n"
+ "int A::A1 = 0;\n"
"\n"
"} // namespace na\n"
"namespace x {\n"
@@ -1005,6 +993,93 @@ TEST_F(ChangeNamespaceTest, TypedefAlias
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
}
+TEST_F(ChangeNamespaceTest, DerivedClassWithConstructors) {
+ std::string Code =
+ "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "class A : public nx::ny::X {\n"
+ "public:\n"
+ " A() : X(0) {}\n"
+ " A(int i);\n"
+ "};\n"
+ "A::A(int i) : X(i) {}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected =
+ "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
+ "\n\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class A : public nx::ny::X {\n"
+ "public:\n"
+ " A() : X(0) {}\n"
+ " A(int i);\n"
+ "};\n"
+ "A::A(int i) : X(i) {}\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, DerivedClassWithQualifiedConstructors) {
+ std::string Code =
+ "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "class A : public nx::ny::X {\n"
+ "public:\n"
+ " A() : X::X(0) {}\n"
+ " A(int i);\n"
+ "};\n"
+ "A::A(int i) : X::X(i) {}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected =
+ "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
+ "\n\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class A : public nx::ny::X {\n"
+ "public:\n"
+ " A() : X::X(0) {}\n"
+ " A(int i);\n"
+ "};\n"
+ "A::A(int i) : X::X(i) {}\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, DerivedClassWithConstructorsAndTypeRefs) {
+ std::string Code =
+ "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "class A : public nx::ny::X {\n"
+ "public:\n"
+ " A() : X(0) {}\n"
+ " A(int i);\n"
+ "};\n"
+ "A::A(int i) : X(i) { X x(1);}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected =
+ "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
+ "\n\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class A : public nx::ny::X {\n"
+ "public:\n"
+ " A() : X(0) {}\n"
+ " A(int i);\n"
+ "};\n"
+ "A::A(int i) : X(i) { nx::ny::X x(1);}\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
} // anonymous namespace
} // namespace change_namespace
} // namespace clang
More information about the cfe-commits
mailing list