r287929 - Do not do raw name replacement when FromDecl is a class forward-declaration.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Nov 25 10:02:49 2016
New Revision: 287929

URL: http://llvm.org/viewvc/llvm-project?rev=287929&view=rev
Log:
Do not do raw name replacement when FromDecl is a class forward-declaration.

Summary:
If the `FromDecl` is a class forward declaration, the reference is
still considered as referring to the original definition given the nature
of forward-declarations, so we can't do a raw name replacement in this case.

Reviewers: bkramer

Subscribers: cfe-commits, klimek

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

Modified:
    cfe/trunk/lib/Tooling/Core/Lookup.cpp
    cfe/trunk/unittests/Tooling/LookupTest.cpp

Modified: cfe/trunk/lib/Tooling/Core/Lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Lookup.cpp?rev=287929&r1=287928&r2=287929&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp Fri Nov 25 10:02:49 2016
@@ -13,6 +13,7 @@
 
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -121,14 +122,20 @@ std::string tooling::replaceNestedName(c
          "Expected fully-qualified name!");
 
   // We can do a raw name replacement when we are not inside the namespace for
-  // the original function and it is not in the global namespace.  The
+  // the original class/function and it is not in the global namespace.  The
   // assumption is that outside the original namespace we must have a using
   // statement that makes this work out and that other parts of this refactor
-  // will automatically fix using statements to point to the new function
+  // will automatically fix using statements to point to the new class/function.
+  // However, if the `FromDecl` is a class forward declaration, the reference is
+  // still considered as referring to the original definition, so we can't do a
+  // raw name replacement in this case.
   const bool class_name_only = !Use;
   const bool in_global_namespace =
       isa<TranslationUnitDecl>(FromDecl->getDeclContext());
-  if (class_name_only && !in_global_namespace &&
+  const bool is_class_forward_decl =
+      isa<CXXRecordDecl>(FromDecl) &&
+      !cast<CXXRecordDecl>(FromDecl)->isCompleteDefinition();
+  if (class_name_only && !in_global_namespace && !is_class_forward_decl &&
       !usingFromDifferentCanonicalNamespace(FromDecl->getDeclContext(),
                                             UseContext)) {
     auto Pos = ReplacementString.rfind("::");

Modified: cfe/trunk/unittests/Tooling/LookupTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/LookupTest.cpp?rev=287929&r1=287928&r2=287929&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/LookupTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp Fri Nov 25 10:02:49 2016
@@ -13,7 +13,9 @@ using namespace clang;
 
 namespace {
 struct GetDeclsVisitor : TestVisitor<GetDeclsVisitor> {
-  std::function<void(CallExpr *)> OnCall;
+  std::function<void(CallExpr *)> OnCall = [&](CallExpr *Expr) {};
+  std::function<void(RecordTypeLoc)> OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+  };
   SmallVector<Decl *, 4> DeclStack;
 
   bool VisitCallExpr(CallExpr *Expr) {
@@ -21,6 +23,11 @@ struct GetDeclsVisitor : TestVisitor<Get
     return true;
   }
 
+  bool VisitRecordTypeLoc(RecordTypeLoc Loc) {
+    OnRecordTypeLoc(Loc);
+    return true;
+  }
+
   bool TraverseDecl(Decl *D) {
     DeclStack.push_back(D);
     bool Ret = TestVisitor::TraverseDecl(D);
@@ -29,7 +36,7 @@ struct GetDeclsVisitor : TestVisitor<Get
   }
 };
 
-TEST(LookupTest, replaceNestedName) {
+TEST(LookupTest, replaceNestedFunctionName) {
   GetDeclsVisitor Visitor;
 
   auto replaceCallExpr = [&](const CallExpr *Expr,
@@ -121,4 +128,37 @@ TEST(LookupTest, replaceNestedName) {
                   "} } }\n");
 }
 
+TEST(LookupTest, replaceNestedClassName) {
+  GetDeclsVisitor Visitor;
+
+  auto replaceRecordTypeLoc = [&](RecordTypeLoc Loc,
+                                  StringRef ReplacementString) {
+    const auto *FD = cast<CXXRecordDecl>(Loc.getDecl());
+    return tooling::replaceNestedName(
+        nullptr, Visitor.DeclStack.back()->getDeclContext(), FD,
+        ReplacementString);
+  };
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+    // Filter Types by name since there are other `RecordTypeLoc` in the test
+    // file.
+    if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+      EXPECT_EQ("x::Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b {\n"
+                  "class Foo;\n"
+                  "namespace c { Foo f();; }\n"
+                  "} }\n");
+
+  Visitor.OnRecordTypeLoc = [&](RecordTypeLoc Type) {
+    // Filter Types by name since there are other `RecordTypeLoc` in the test
+    // file.
+    // `a::b::Foo` in using shadow decl is not `TypeLoc`.
+    if (Type.getDecl()->getQualifiedNameAsString() == "a::b::Foo")
+      EXPECT_EQ("Bar", replaceRecordTypeLoc(Type, "::a::x::Bar"));
+  };
+  Visitor.runOver("namespace a { namespace b { class Foo {}; } }\n"
+                  "namespace c { using a::b::Foo; Foo f();; }\n");
+}
+
 } // end anonymous namespace