[clang-tools-extra] r293182 - [change-namespace] add leading '::' to references in new namespace when name conflict is possible.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 26 07:08:44 PST 2017


Author: ioeric
Date: Thu Jan 26 09:08:44 2017
New Revision: 293182

URL: http://llvm.org/viewvc/llvm-project?rev=293182&view=rev
Log:
[change-namespace] add leading '::' to references in new namespace when name conflict is possible.

Summary:
For example, when we change 'na' to "nb::nc", we need to add leading '::' to
references "::nc::X" in the changed namespace.

Reviewers: bkramer

Reviewed By: bkramer

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D29176

Modified:
    clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
    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=293182&r1=293181&r2=293182&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Thu Jan 26 09:08:44 2017
@@ -196,6 +196,8 @@ tooling::Replacement createInsertion(Sou
 // 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 @@ std::string getShortestQualifiedNameInNa
   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<llvm::StringRef, 4> NsNameSplitted;
+  NsName.split(NsNameSplitted, "::", /*MaxSplit=*/-1,
+               /*KeepEmpty=*/false);
+  llvm::SmallVector<llvm::StringRef, 4> 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;
+  }
+  // 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 DeclName;
+  return (DeclI == DeclE)
+             ? UnqualifiedDeclName.str()
+             : (llvm::join(DeclI, DeclE, "::") + "::" + UnqualifiedDeclName)
+                   .str();
 }
 
 std::string wrapCodeInNamespace(StringRef NestedNs, std::string Code) {

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=293182&r1=293181&r2=293182&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp Thu Jan 26 09:08:44 2017
@@ -1599,6 +1599,74 @@ TEST_F(ChangeNamespaceTest, TemplateUsin
   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




More information about the cfe-commits mailing list