Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.h =================================================================== --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.h +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.h @@ -145,6 +145,9 @@ // Records all using namespace declarations, which can be used to shorten // namespace specifiers. llvm::SmallPtrSet UsingNamespaceDecls; + // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need to + // be fixed. + llvm::SmallVector BaseCtorInitializerTypeLocs; }; } // namespace change_namespace Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp =================================================================== --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp @@ -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 @@ .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 @@ SourceLocation Start = Specifier->getBeginLoc(); SourceLocation End = EndLocationForType(Specifier->getTypeLoc()); fixTypeLoc(Result, Start, End, Specifier->getTypeLoc()); + } else if (const auto *BaseInitializer = + Result.Nodes.getNodeAs( + "base_initializer")) { + BaseCtorInitializerTypeLocs.push_back( + BaseInitializer->getTypeSourceInfo()->getTypeLoc()); } else if (const auto *TLoc = Result.Nodes.getNodeAs("type")) { fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc), *TLoc); @@ -520,7 +539,9 @@ // 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 @@ 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 @@ // 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("from_decl"); // `hasDeclaration` gives underlying declaration, but if the type is @@ -667,7 +693,9 @@ // 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() { Index: clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp =================================================================== --- clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp +++ clang-tools-extra/trunk/test/change-namespace/lambda-function.cpp @@ -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 function; +template +class function { +public: + template + function(Functor f) {} + R operator()(ArgTypes...) const {} +}; + +// CHECK: namespace x { +// CHECK-NEXT: namespace y { +namespace na { +namespace nb { +void f(function func, int param) { func(param); } +void g() { f([](int x) {}, 1); } +// CHECK: } // namespace y +// CHECK-NEXT: } // namespace x +} +} Index: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp +++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp @@ -43,8 +43,9 @@ NamespaceTool.registerMatchers(&Finder); std::unique_ptr 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 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } -TEST_F(ChangeNamespaceTest, DoNotCrashWithLambdaAsParameter) { - std::string Code = - "#include \n" - "void f(std::function func, int param) { func(param); } " - "void g() { f([](int x) {}, 1); }"; - - std::string Expected = - "#include \n" - "void f(std::function 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 @@ "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 @@ "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 @@ 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