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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
