Fully qualify the renamed symbol if the shortened name is ambiguous.

Summary:
For example, when renaming `a::b::x::foo` to `y::foo` below, replacing
`x::foo()` with `y::foo()` can cause ambiguity. In such cases, we simply fully
qualify the name with leading `::`.
```
namespace a {
namespace b {
namespace x { void foo() {} }
namespace y { void foo() {} }
}
}

namespace a {
namespace b {
void f() { x::foo(); }
}
}
```

Reviewers: ilya-biryukov, hokein

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

llvm-svn: 338832

1 files changed, 42 insertions, 2 deletions

diff --git a/clang/lib/Tooling/Core/Lookup.cpp b/clang/lib/Tooling/Core/Lookup.cpp
index 6edf61b8050..cc448d144e2 100644
--- a/clang/lib/Tooling/Core/Lookup.cpp
+++ b/clang/lib/Tooling/Core/Lookup.cpp
@@ -14,6 +14,7 @@
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -114,6 +115,37 @@ static bool isFullyQualified(const NestedNameSpecifier *NNS) {
   return false;
 }
 
+// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is
+// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar"
+// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar"
+// can be resolved to ::a::y::bar, which can cause compile error.
+// FIXME: consider using namespaces.
+static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName,
+                                   const DeclContext &UseContext) {
+  assert(QName.startswith("::"));
+  if (Spelling.startswith("::"))
+    return false;
+
+  // Lookup the first component of Spelling in all enclosing namespaces and
+  // check if there is any existing symbols with the same name but in different
+  // scope.
+  StringRef Head = Spelling.split("::").first;
+
+  llvm::SmallVector<const NamespaceDecl *, 4> UseNamespaces =
+      getAllNamedNamespaces(&UseContext);
+  auto &AST = UseContext.getParentASTContext();
+  StringRef TrimmedQName = QName.substr(2);
+  for (const auto *NS : UseNamespaces) {
+    auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+    if (!LookupRes.empty()) {
+      for (const NamedDecl *Res : LookupRes)
+        if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
+          return true;
+    }
+  }
+  return false;
+}
+
 std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
                                        const DeclContext *UseContext,
                                        const NamedDecl *FromDecl,
@@ -146,6 +178,14 @@ std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
   // figure out how good a namespace match we have with our destination type.
   // We work backwards (from most specific possible namespace to least
   // specific).
-  return getBestNamespaceSubstr(UseContext, ReplacementString,
-                                isFullyQualified(Use));
+  StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString,
+                                               isFullyQualified(Use));
+  // Use the fully qualified name if the suggested name is ambiguous.
+  // FIXME: consider re-shortening the name until the name is not ambiguous. We
+  // are not doing this because ambiguity is pretty bad and we should not try to
+  // be clever in handling such cases. Making this noticeable to users seems to
+  // be a better option.
+  return isAmbiguousNameInScope(Suggested, ReplacementString, *UseContext)
+             ? ReplacementString
+             : Suggested;
 }