[change-namespace] avoid adding leading '::' when possible.

Summary:
When changing namespaces, the tool adds leading "::" to references that need to
be fully-qualified, which would affect readability.

We avoid adding "::" when the symbol name does not conflict with the new
namespace name. For example, a symbol name "na::nb::X" conflicts with "ns::na"
since it would be resolved to "ns::na::nb::X" in the new namespace.

Reviewers: hokein

Reviewed By: hokein

Subscribers: cfe-commits

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

1 files changed, 38 insertions, 17 deletions

diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.cpp b/clang-tools-extra/change-namespace/ChangeNamespace.cpp
index e07066794b5..0b90de414c5 100644
--- a/clang-tools-extra/change-namespace/ChangeNamespace.cpp
+++ b/clang-tools-extra/change-namespace/ChangeNamespace.cpp
@@ -28,6 +28,14 @@ joinNamespaces(const llvm::SmallVectorImpl<StringRef> &Namespaces) {
   return Result;
 }
 
+// Given "a::b::c", returns {"a", "b", "c"}.
+llvm::SmallVector<llvm::StringRef, 4> splitSymbolName(llvm::StringRef Name) {
+  llvm::SmallVector<llvm::StringRef, 4> Splitted;
+  Name.split(Splitted, "::", /*MaxSplit=*/-1,
+             /*KeepEmpty=*/false);
+  return Splitted;
+}
+
 SourceLocation startLocationForType(TypeLoc TLoc) {
   // For elaborated types (e.g. `struct a::A`) we want the portion after the
   // `struct` but including the namespace qualifier, `a::`.
@@ -68,9 +76,7 @@ const NamespaceDecl *getOuterNamespace(const NamespaceDecl *InnerNs,
     return nullptr;
   const auto *CurrentContext = llvm::cast<DeclContext>(InnerNs);
   const auto *CurrentNs = InnerNs;
-  llvm::SmallVector<llvm::StringRef, 4> PartialNsNameSplitted;
-  PartialNsName.split(PartialNsNameSplitted, "::", /*MaxSplit=*/-1,
-                      /*KeepEmpty=*/false);
+  auto PartialNsNameSplitted = splitSymbolName(PartialNsName);
   while (!PartialNsNameSplitted.empty()) {
     // Get the inner-most namespace in CurrentContext.
     while (CurrentContext && !llvm::isa<NamespaceDecl>(CurrentContext))
@@ -208,12 +214,8 @@ std::string getShortestQualifiedNameInNamespace(llvm::StringRef DeclName,
   if (DeclName.find(':') == llvm::StringRef::npos)
     return DeclName;
 
-  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);
+  auto NsNameSplitted = splitSymbolName(NsName);
+  auto DeclNsSplitted = splitSymbolName(DeclName);
   llvm::StringRef UnqualifiedDeclName = DeclNsSplitted.pop_back_val();
   // If the Decl is in global namespace, there is no need to shorten it.
   if (DeclNsSplitted.empty())
@@ -249,9 +251,7 @@ std::string getShortestQualifiedNameInNamespace(llvm::StringRef DeclName,
 std::string wrapCodeInNamespace(StringRef NestedNs, std::string Code) {
   if (Code.back() != '\n')
     Code += "\n";
-  llvm::SmallVector<StringRef, 4> NsSplitted;
-  NestedNs.split(NsSplitted, "::", /*MaxSplit=*/-1,
-                 /*KeepEmpty=*/false);
+  auto NsSplitted = splitSymbolName(NestedNs);
   while (!NsSplitted.empty()) {
     // FIXME: consider code style for comments.
     Code = ("namespace " + NsSplitted.back() + " {\n" + Code +
@@ -282,6 +282,28 @@ bool isDeclVisibleAtLocation(const SourceManager &SM, const Decl *D,
           isNestedDeclContext(DeclCtx, D->getDeclContext()));
 }
 
+// Given a qualified symbol name, returns true if the symbol will be
+// incorrectly qualified without leading "::".
+bool conflictInNamespace(llvm::StringRef QualifiedSymbol,
+                         llvm::StringRef Namespace) {
+  auto SymbolSplitted = splitSymbolName(QualifiedSymbol.trim(":"));
+  assert(!SymbolSplitted.empty());
+  SymbolSplitted.pop_back();  // We are only interested in namespaces.
+
+  if (SymbolSplitted.size() > 1 && !Namespace.empty()) {
+    auto NsSplitted = splitSymbolName(Namespace.trim(":"));
+    assert(!NsSplitted.empty());
+    // We do not check the outermost namespace since it would not be a conflict
+    // if it equals to the symbol's outermost namespace and the symbol name
+    // would have been shortened.
+    for (auto I = NsSplitted.begin() + 1, E = NsSplitted.end(); I != E; ++I) {
+      if (*I == SymbolSplitted.front())
+        return true;
+    }
+  }
+  return false;
+}
+
 AST_MATCHER(EnumDecl, isScoped) {
     return Node.isScoped();
 }
@@ -306,10 +328,8 @@ ChangeNamespaceTool::ChangeNamespaceTool(
       OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')),
       FilePattern(FilePattern), FilePatternRE(FilePattern) {
   FileToReplacements->clear();
-  llvm::SmallVector<llvm::StringRef, 4> OldNsSplitted;
-  llvm::SmallVector<llvm::StringRef, 4> NewNsSplitted;
-  llvm::StringRef(OldNamespace).split(OldNsSplitted, "::");
-  llvm::StringRef(NewNamespace).split(NewNsSplitted, "::");
+  auto OldNsSplitted = splitSymbolName(OldNamespace);
+  auto NewNsSplitted = splitSymbolName(NewNamespace);
   // Calculates `DiffOldNamespace` and `DiffNewNamespace`.
   while (!OldNsSplitted.empty() && !NewNsSplitted.empty() &&
          OldNsSplitted.front() == NewNsSplitted.front()) {
@@ -825,7 +845,8 @@ void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext(
     return;
   // If the reference need to be fully-qualified, add a leading "::" unless
   // NewNamespace is the global namespace.
-  if (ReplaceName == FromDeclName && !NewNamespace.empty())
+  if (ReplaceName == FromDeclName && !NewNamespace.empty() &&
+      conflictInNamespace(ReplaceName, NewNamespace))
     ReplaceName = "::" + ReplaceName;
   addReplacementOrDie(Start, End, ReplaceName, *Result.SourceManager,
                       &FileToReplacements);