[Lookup] Invisible decls should not be ambiguous when renaming.

Summary:
For example, a renamed type in a header file can conflict with declaration in
a random file that includes the header, but we should not consider the decl ambiguous if
it's not visible at the rename location. This improves consistency of generated replacements
when header file is included in different TUs.

Reviewers: hokein

Subscribers: cfe-commits

Tags: #clang

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

llvm-svn: 358378

1 files changed, 40 insertions, 6 deletions

diff --git a/clang/unittests/Tooling/LookupTest.cpp b/clang/unittests/Tooling/LookupTest.cpp
index 2a4893e90cc..d79e1746a03 100644
--- a/clang/unittests/Tooling/LookupTest.cpp
+++ b/clang/unittests/Tooling/LookupTest.cpp
@@ -44,8 +44,8 @@ TEST(LookupTest, replaceNestedFunctionName) {
     const auto *Callee = cast<DeclRefExpr>(Expr->getCallee()->IgnoreImplicit());
     const ValueDecl *FD = Callee->getDecl();
     return tooling::replaceNestedName(
-        Callee->getQualifier(), Visitor.DeclStack.back()->getDeclContext(), FD,
-        ReplacementString);
+        Callee->getQualifier(), Callee->getLocation(),
+        Visitor.DeclStack.back()->getDeclContext(), FD, ReplacementString);
   };
 
   Visitor.OnCall = [&](CallExpr *Expr) {
@@ -181,12 +181,12 @@ TEST(LookupTest, replaceNestedFunctionName) {
 TEST(LookupTest, replaceNestedClassName) {
   GetDeclsVisitor Visitor;
 
-  auto replaceRecordTypeLoc = [&](RecordTypeLoc Loc,
+  auto replaceRecordTypeLoc = [&](RecordTypeLoc TLoc,
                                   StringRef ReplacementString) {
-    const auto *FD = cast<CXXRecordDecl>(Loc.getDecl());
+    const auto *FD = cast<CXXRecordDecl>(TLoc.getDecl());
     return tooling::replaceNestedName(
-        nullptr, Visitor.DeclStack.back()->getDeclContext(), FD,
-        ReplacementString);
+        nullptr, TLoc.getBeginLoc(), Visitor.DeclStack.back()->getDeclContext(),
+        FD, ReplacementString);
   };
 
   Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
@@ -211,6 +211,40 @@ TEST(LookupTest, replaceNestedClassName) {
   };
   Visitor.runOver("namespace a { namespace b { class Foo {}; } }\n"
                   "namespace c { using a::b::Foo; Foo f();; }\n");
+
+  // Rename TypeLoc `x::y::Old` to new name `x::Foo` at [0] and check that the
+  // type is replaced with "Foo" instead of "x::Foo". Although there is a symbol
+  // `x::y::Foo` in c.cc [1], it should not make "Foo" at [0] ambiguous because
+  // it's not visible at [0].
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+    if (Type.getDecl()->getQualifiedNameAsString() == "x::y::Old")
+      EXPECT_EQ("Foo", replaceRecordTypeLoc(Type, "::x::Foo"));
+  };
+  Visitor.runOver(R"(
+    // a.h
+    namespace x {
+     namespace y {
+      class Old {};
+      class Other {};
+     }
+    }
+
+    // b.h
+    namespace x {
+     namespace y {
+      // This is to be renamed to x::Foo
+      // The expected replacement is "Foo".
+      Old f;  // [0].
+     }
+    }
+
+    // c.cc
+    namespace x {
+    namespace y {
+     using Foo = ::x::y::Other; // [1]
+    }
+    }
+    )");
 }
 
 } // end anonymous namespace