From 97f87add50e9b435a104449372fdd08a8f8723b7 Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Wed, 7 Dec 2016 20:08:02 +0000 Subject: [PATCH] [change-namespace] always add a '::' prefix when a symbol reference needs to be fully-qualified. llvm-svn: 288969 --- .../change-namespace/ChangeNamespace.cpp | 4 + .../change-namespace/ChangeNamespace.h | 5 +- .../change-namespace/ChangeNamespaceTests.cpp | 143 ++++++++++++++++----- 3 files changed, 121 insertions(+), 31 deletions(-) diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.cpp b/clang-tools-extra/change-namespace/ChangeNamespace.cpp index 45064eb..2dca1a9 100644 --- a/clang-tools-extra/change-namespace/ChangeNamespace.cpp +++ b/clang-tools-extra/change-namespace/ChangeNamespace.cpp @@ -663,6 +663,10 @@ void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext( // old namespace, we don't create replacement. if (NestedName == ReplaceName) return; + // If the reference need to be fully-qualified, add a leading "::" unless + // NewNamespace is the global namespace. + if (ReplaceName == FromDeclName && !NewNamespace.empty()) + ReplaceName = "::" + ReplaceName; auto R = createReplacement(Start, End, ReplaceName, *Result.SourceManager); auto Err = FileToReplacements[R.getFilePath()].add(R); if (Err) diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.h b/clang-tools-extra/change-namespace/ChangeNamespace.h index 048c32f..342457a 100644 --- a/clang-tools-extra/change-namespace/ChangeNamespace.h +++ b/clang-tools-extra/change-namespace/ChangeNamespace.h @@ -24,6 +24,9 @@ namespace change_namespace { // namespaces while references to symbols (e.g. types, functions) which are not // defined in the changed namespace will be correctly qualified by prepending // namespace specifiers before them. +// This will try to add shortest namespace specifiers possible. When a symbol +// reference needs to be fully-qualified, this adds a "::" prefix to the +// namespace specifiers unless the new namespace is the global namespace. // For classes, only classes that are declared/defined in the given namespace in // speficifed files will be moved: forward declarations will remain in the old // namespace. @@ -38,7 +41,7 @@ namespace change_namespace { // class FWD; // } // a // namespace x { -// class A { a::FWD *fwd; } +// class A { ::a::FWD *fwd; } // } // x // FIXME: support moving typedef, enums across namespaces. class ChangeNamespaceTool : public ast_matchers::MatchFinder::MatchCallback { diff --git a/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp b/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp index 9beeced..8ed0f9d 100644 --- a/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp +++ b/clang-tools-extra/unittests/change-namespace/ChangeNamespaceTests.cpp @@ -174,6 +174,40 @@ TEST_F(ChangeNamespaceTest, MoveIntoAnotherNestedNamespaceWithRef) { EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, MoveIntoExistingNamespaceAndShortenRefs) { + std::string Code = "namespace x {\n" + "namespace z {\n" + "class Z {};\n" + "} // namespace z\n" + "namespace y {\n" + "class T {};\n" + "} // namespace y\n" + "} // namespace x\n" + "namespace na {\n" + "class A{};\n" + "namespace nb {\n" + "class X { A a; x::z::Z zz; x::y::T t; };\n" + "} // namespace nb\n" + "} // namespace na\n"; + std::string Expected = "namespace x {\n" + "namespace z {\n" + "class Z {};\n" + "} // namespace z\n" + "namespace y {\n" + "class T {};\n" + "} // namespace y\n" + "} // namespace x\n" + "namespace na {\n" + "class A {};\n\n" + "} // namespace na\n" + "namespace x {\n" + "namespace y {\n" + "class X { ::na::A a; z::Z zz; T t; };\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + TEST_F(ChangeNamespaceTest, SimpleMoveNestedNamespace) { NewNamespace = "na::x::y"; std::string Code = "namespace na {\n" @@ -222,8 +256,8 @@ TEST_F(ChangeNamespaceTest, SimpleMoveWithTypeRefs) { "namespace y {\n" "class C_X {\n" "public:\n" - " na::C_A a;\n" - " na::nc::C_C c;\n" + " ::na::C_A a;\n" + " ::na::nc::C_C c;\n" "};\n" "class C_Y {\n" " C_X x;\n" @@ -265,9 +299,9 @@ TEST_F(ChangeNamespaceTest, TypeLocInTemplateSpecialization) { "namespace x {\n" "namespace y {\n" "void f() {\n" - " na::B b;\n" - " na::B b_c;\n" - " na::Two two;\n" + " ::na::B<::na::A> b;\n" + " ::na::B<::na::nc::C> b_c;\n" + " ::na::Two<::na::A, ::na::nc::C> two;\n" "}\n" "} // namespace y\n" "} // namespace x\n"; @@ -294,7 +328,7 @@ TEST_F(ChangeNamespaceTest, LeaveForwardDeclarationBehind) { "namespace y {\n" "\n" "class A {\n" - " na::nb::FWD *fwd;\n" + " ::na::nb::FWD *fwd;\n" "};\n" "} // namespace y\n" "} // namespace x\n"; @@ -322,7 +356,7 @@ TEST_F(ChangeNamespaceTest, TemplateClassForwardDeclaration) { "namespace y {\n" "\n" "class A {\n" - " na::nb::FWD *fwd;\n" + " ::na::nb::FWD *fwd;\n" "};\n" "template class TEMP {};\n" "} // namespace y\n" @@ -377,8 +411,8 @@ TEST_F(ChangeNamespaceTest, MoveFunctions) { "namespace x {\n" "namespace y {\n" "void fwd();\n" - "void f(na::C_A ca, na::nc::C_C cc) {\n" - " na::C_A ca_1 = ca;\n" + "void f(::na::C_A ca, ::na::nc::C_C cc) {\n" + " ::na::C_A ca_1 = ca;\n" "}\n" "} // namespace y\n" "} // namespace x\n"; @@ -417,9 +451,9 @@ TEST_F(ChangeNamespaceTest, FixUsingShadowDecl) { "namespace x {\n" "namespace y {\n" "using ::na::nc::SAME;\n" - "using YO = na::nd::SAME;\n" - "typedef na::nd::SAME IDENTICAL;\n" - "void f(na::nd::SAME Same) {}\n" + "using YO = ::na::nd::SAME;\n" + "typedef ::na::nd::SAME IDENTICAL;\n" + "void f(::na::nd::SAME Same) {}\n" "} // namespace y\n" "} // namespace x\n"; EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); @@ -445,9 +479,9 @@ TEST_F(ChangeNamespaceTest, DontFixUsingShadowDeclInClasses) { "} // namespace na\n" "namespace x {\n" "namespace y {\n" - "class D : public na::Base {\n" + "class D : public ::na::Base {\n" "public:\n" - " using AA = na::A; using B = na::Base;\n" + " using AA = ::na::A; using B = ::na::Base;\n" " using Base::m; using Base::Base;\n" "};" "} // namespace y\n" @@ -492,11 +526,11 @@ TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) { "namespace x {\n" "namespace y {\n" "class C_X {\n" - " na::C_A na;\n" - " na::C_A::Nested nested;\n" + " ::na::C_A na;\n" + " ::na::C_A::Nested nested;\n" " void f() {\n" - " na::C_A::Nested::nestedFunc();\n" - " int X = na::C_A::Nested::NestedX;\n" + " ::na::C_A::Nested::nestedFunc();\n" + " int X = ::na::C_A::Nested::NestedX;\n" " }\n" "};\n" "} // namespace y\n" @@ -534,8 +568,8 @@ TEST_F(ChangeNamespaceTest, FixFunctionNameSpecifiers) { "} // namespace na\n" "namespace x {\n" "namespace y {\n" - "void f() { na::a_f(); na::static_f(); na::A::f(); }\n" - "void g() { f(); na::A::g(); }\n" + "void f() { ::na::a_f(); ::na::static_f(); ::na::A::f(); }\n" + "void g() { f(); ::na::A::g(); }\n" "} // namespace y\n" "} // namespace x\n"; EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); @@ -551,7 +585,9 @@ TEST_F(ChangeNamespaceTest, FixNonCallingFunctionReferences) { "static void s_f() {}\n" "namespace nb {\n" "void f() {\n" - "auto *ref1 = A::f; auto *ref2 = a_f; auto *ref3 = s_f;\n" + " auto *ref1 = A::f;\n" + " auto *ref2 = a_f;\n" + " auto *ref3 = s_f;\n" "}\n" "} // namespace nb\n" "} // namespace na\n"; @@ -568,7 +604,9 @@ TEST_F(ChangeNamespaceTest, FixNonCallingFunctionReferences) { "namespace x {\n" "namespace y {\n" "void f() {\n" - "auto *ref1 = na::A::f; auto *ref2 = na::a_f; auto *ref3 = na::s_f;\n" + " auto *ref1 = ::na::A::f;\n" + " auto *ref2 = ::na::a_f;\n" + " auto *ref3 = ::na::s_f;\n" "}\n" "} // namespace y\n" "} // namespace x\n"; @@ -600,9 +638,9 @@ TEST_F(ChangeNamespaceTest, MoveAndFixGlobalVariables) { "namespace y {\n" "int GlobB;\n" "void f() {\n" - " int a = na::GlobA;\n" - " int b = na::GlobAStatic;\n" - " int c = na::nc::GlobC;\n" + " int a = ::na::GlobA;\n" + " int b = ::na::GlobAStatic;\n" + " int c = ::na::nc::GlobC;\n" "}\n" "} // namespace y\n" "} // namespace x\n"; @@ -619,7 +657,9 @@ TEST_F(ChangeNamespaceTest, DoNotFixStaticVariableOfClass) { "};\n" "int A::A1 = 0;\n" "namespace nb {\n" - "void f() { int a = A::A1; int b = A::A2; }" + "void f() {\n" + " int a = A::A1; int b = A::A2;\n" + "}\n" "} // namespace nb\n" "} // namespace na\n"; @@ -634,7 +674,9 @@ TEST_F(ChangeNamespaceTest, DoNotFixStaticVariableOfClass) { "} // namespace na\n" "namespace x {\n" "namespace y {\n" - "void f() { int a = na::A::A1; int b = na::A::A2; }" + "void f() {\n" + " int a = ::na::A::A1; int b = ::na::A::A2;\n" + "}\n" "} // namespace y\n" "} // namespace x\n"; @@ -966,7 +1008,7 @@ TEST_F(ChangeNamespaceTest, UsingDeclInMovedNamespace) { "} // na\n" "namespace x {\n" "namespace y {\n" - "void d() { nx::f(); }\n" + "void d() { ::nx::f(); }\n" "} // namespace y\n" "} // namespace x\n"; EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); @@ -1017,7 +1059,7 @@ TEST_F(ChangeNamespaceTest, UsingDeclInMovedNamespaceMultiNested) { "} // c\n" "namespace x {\n" "namespace y {\n" - "void d() { f(); nx::g(); }\n" + "void d() { f(); ::nx::g(); }\n" "} // namespace y\n" "} // namespace x\n" "} // b\n" @@ -1192,7 +1234,7 @@ TEST_F(ChangeNamespaceTest, DerivedClassWithConstructorsAndTypeRefs) { " A() : X(0) {}\n" " A(int i);\n" "};\n" - "A::A(int i) : X(i) { nx::ny::X x(1);}\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)); @@ -1234,6 +1276,47 @@ TEST_F(ChangeNamespaceTest, MoveToGlobalNamespace) { EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, KeepGlobalSpecifier) { + std::string Code = "class Glob {};\n" + "namespace na {\n" + "class C_A {};\n" + "namespace nc {\n" + "class C_C {};" + "} // namespace nc\n" + "namespace nb {\n" + "class C_X {\n" + "public:\n" + " ::Glob glob_1;\n" + " Glob glob_2;\n" + " C_A a_1;\n" + " ::na::C_A a_2;\n" + " nc::C_C c;\n" + "};\n" + "} // namespace nb\n" + "} // namespace na\n"; + std::string Expected = "class Glob {};\n" + "namespace na {\n" + "class C_A {};\n" + "namespace nc {\n" + "class C_C {};" + "} // namespace nc\n" + "\n" + "} // namespace na\n" + "namespace x {\n" + "namespace y {\n" + "class C_X {\n" + "public:\n" + " ::Glob glob_1;\n" + " Glob glob_2;\n" + " ::na::C_A a_1;\n" + " ::na::C_A a_2;\n" + " ::na::nc::C_C c;\n" + "};\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + } // anonymous namespace } // namespace change_namespace } // namespace clang -- 2.7.4