ioeric created this revision. ioeric added a reviewer: bkramer. ioeric added a subscriber: cfe-commits. Herald added a subscriber: klimek.
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. https://reviews.llvm.org/D27132 Files: lib/Tooling/Core/Lookup.cpp unittests/Tooling/LookupTest.cpp
Index: unittests/Tooling/LookupTest.cpp =================================================================== --- unittests/Tooling/LookupTest.cpp +++ unittests/Tooling/LookupTest.cpp @@ -13,23 +13,30 @@ 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) { OnCall(Expr); return true; } + bool VisitRecordTypeLoc(RecordTypeLoc Loc) { + OnRecordTypeLoc(Loc); + return true; + } + bool TraverseDecl(Decl *D) { DeclStack.push_back(D); bool Ret = TestVisitor::TraverseDecl(D); DeclStack.pop_back(); return Ret; } }; -TEST(LookupTest, replaceNestedName) { +TEST(LookupTest, replaceNestedFunctionName) { GetDeclsVisitor Visitor; auto replaceCallExpr = [&](const CallExpr *Expr, @@ -121,4 +128,37 @@ "} } }\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 Index: lib/Tooling/Core/Lookup.cpp =================================================================== --- lib/Tooling/Core/Lookup.cpp +++ lib/Tooling/Core/Lookup.cpp @@ -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,21 @@ "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 given the nature + // of forward-declarations, 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("::");
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits