This revision was automatically updated to reflect the committed changes. Closed by commit rL287929: Do not do raw name replacement when FromDecl is a class forward-declaration. (authored by ioeric).
Changed prior to commit: https://reviews.llvm.org/D27132?vs=79307&id=79308#toc Repository: rL LLVM https://reviews.llvm.org/D27132 Files: cfe/trunk/lib/Tooling/Core/Lookup.cpp cfe/trunk/unittests/Tooling/LookupTest.cpp
Index: cfe/trunk/unittests/Tooling/LookupTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/LookupTest.cpp +++ cfe/trunk/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: cfe/trunk/lib/Tooling/Core/Lookup.cpp =================================================================== --- cfe/trunk/lib/Tooling/Core/Lookup.cpp +++ cfe/trunk/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,20 @@ "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("::");
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits