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 @@ -196,6 +196,8 @@ // Returns the shortest qualified name for declaration `DeclName` in the // namespace `NsName`. For example, if `DeclName` is "a::b::X" and `NsName` // is "a::c::d", then "b::X" will be returned. +// Note that if `DeclName` is `::b::X` and `NsName` is `::a::b`, this returns +// "::b::X" instead of "b::X" since there will be a name conflict otherwise. // \param DeclName A fully qualified name, "::a::b::X" or "a::b::X". // \param NsName A fully qualified name, "::a::b" or "a::b". Global namespace // will have empty name. @@ -206,14 +208,42 @@ if (DeclName.find(':') == llvm::StringRef::npos) return DeclName; - while (!DeclName.consume_front((NsName + "::").str())) { - const auto Pos = NsName.find_last_of(':'); - if (Pos == llvm::StringRef::npos) - return DeclName; - assert(Pos > 0); - NsName = NsName.substr(0, Pos - 1); + llvm::SmallVector NsNameSplitted; + NsName.split(NsNameSplitted, "::", /*MaxSplit=*/-1, + /*KeepEmpty=*/false); + llvm::SmallVector DeclNsSplitted; + DeclName.split(DeclNsSplitted, "::", /*MaxSplit=*/-1, + /*KeepEmpty=*/false); + llvm::StringRef UnqualifiedDeclName = DeclNsSplitted.pop_back_val(); + // If the Decl is in global namespace, there is no need to shorten it. + if (DeclNsSplitted.empty()) + return UnqualifiedDeclName; + // If NsName is the global namespace, we can simply use the DeclName sans + // leading "::". + if (NsNameSplitted.empty()) + return DeclName; + + if (NsNameSplitted.front() != DeclNsSplitted.front()) { + // The DeclName must be fully-qualified, but we still need to decide if a + // leading "::" is necessary. For example, if `NsName` is "a::b::c" and the + // `DeclName` is "b::X", then the reference must be qualified as "::b::X" + // to avoid conflict. + if (llvm::is_contained(NsNameSplitted, DeclNsSplitted.front())) + return ("::" + DeclName).str(); + return DeclName; } - return DeclName; + // Since there is already an overlap namespace, we know that `DeclName` can be + // shortened, so we reduce the longest common prefix. + auto DeclI = DeclNsSplitted.begin(); + auto DeclE = DeclNsSplitted.end(); + auto NsI = NsNameSplitted.begin(); + auto NsE = NsNameSplitted.end(); + for (; DeclI != DeclE && NsI != NsE && *DeclI == *NsI; ++DeclI, ++NsI) { + } + return (DeclI == DeclE) + ? UnqualifiedDeclName.str() + : (llvm::join(DeclI, DeclE, "::") + "::" + UnqualifiedDeclName) + .str(); } std::string wrapCodeInNamespace(StringRef NestedNs, std::string Code) { 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 @@ -1599,6 +1599,74 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, ExistingNamespaceConflictWithNewNamespace) { + OldNamespace = "nx"; + NewNamespace = "ny::na::nc"; + std::string Code = "namespace na {\n" + "class A {};\n" + "} // namespace na\n" + "namespace nb {\n" + "class B {};\n" + "} // namespace nb\n" + "namespace nx {\n" + "class X {\n" + " na::A a; nb::B b;\n" + "};\n" + "} // namespace nx\n"; + std::string Expected = "namespace na {\n" + "class A {};\n" + "} // namespace na\n" + "namespace nb {\n" + "class B {};\n" + "} // namespace nb\n" + "\n" + "namespace ny {\n" + "namespace na {\n" + "namespace nc {\n" + "class X {\n" + " ::na::A a; nb::B b;\n" + "};\n" + "} // namespace nc\n" + "} // namespace na\n" + "} // namespace ny\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, ShortenNamespaceSpecifier) { + OldNamespace = "nx"; + NewNamespace = "ny::na"; + std::string Code = "class G {};\n" + "namespace ny {\n" + "class Y {};\n" + "namespace na {\n" + "class A {};\n" + "namespace nc { class C {}; } // namespace nc\n" + "}\n // namespace na\n" + "}\n // namespace ny\n" + "namespace nx {\n" + "class X {\n" + " G g; ny::Y y; ny::na::A a; ny::na::nc::C c;\n" + "};\n" + "} // namespace nx\n"; + std::string Expected = "class G {};\n" + "namespace ny {\n" + "class Y {};\n" + "namespace na {\n" + "class A {};\n" + "namespace nc { class C {}; } // namespace nc\n" + "}\n // namespace na\n" + "}\n // namespace ny\n" + "\n" + "namespace ny {\n" + "namespace na {\n" + "class X {\n" + " G g; Y y; A a; nc::C c;\n" + "};\n" + "} // namespace na\n" + "} // namespace ny\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + } // anonymous namespace } // namespace change_namespace } // namespace clang