kbobyrev updated this revision to Diff 304729. kbobyrev added a comment. Use early return in FieldDecl canonicalization.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/RenameTests.cpp +++ clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -588,6 +588,147 @@ ns::[[Old^Alias]] Bar; } )cpp", + + // Templated method instantiation. + R"cpp( + template<typename T> + class Foo { + public: + static T [[f^oo]]() {} + }; + + void bar() { + Foo<int>::[[f^oo]](); + } + )cpp", + R"cpp( + template<typename T> + class Foo { + public: + T [[f^oo]]() {} + }; + + void bar() { + Foo<int>().[[f^oo]](); + } + )cpp", + + // Templated class specialization. + R"cpp( + template<typename T, typename U=bool> + class [[Foo^]]; + + template<typename T, typename U> + class [[Foo^]] {}; + + template<typename T=int, typename U> + class [[Foo^]]; + )cpp", + R"cpp( + template<typename T=float, typename U=int> + class [[Foo^]]; + + template<typename T, typename U> + class [[Foo^]] {}; + )cpp", + + // Function template specialization. + R"cpp( + template<typename T=int, typename U=bool> + U [[foo^]](); + + template<typename T, typename U> + U [[foo^]]() {}; + )cpp", + R"cpp( + template<typename T, typename U> + U [[foo^]]() {}; + + template<typename T=int, typename U=bool> + U [[foo^]](); + )cpp", + R"cpp( + template<typename T=int, typename U=bool> + U [[foo^]](); + + template<typename T, typename U> + U [[foo^]](); + )cpp", + + // Fields in classes & partial and full specialiations. + R"cpp( + class Foo { + public: + Foo(int Variable) : [[Variabl^e]](Variable) {} + + int [[Va^riable]] = 42; + + private: + void foo() { ++[[Vari^able]]; } + }; + + void bar() { + Foo f(9000); + f.[[Variable^]] = -1; + } + )cpp", + R"cpp( + template<typename T> + struct Foo { + T [[Vari^able]] = 42; + }; + + void foo() { + Foo<int> f; + f.[[Varia^ble]] = 9000; + } + )cpp", + R"cpp( + template<typename T, typename U> + struct Foo { + T Variable[42]; + U Another; + + void bar() {} + }; + + template<typename T> + struct Foo<T, bool> { + T [[Var^iable]]; + void bar() { ++[[Var^iable]]; } + }; + + void foo() { + Foo<unsigned, bool> f; + f.[[Var^iable]] = 9000; + } + )cpp", + R"cpp( + template<typename T, typename U> + struct Foo { + T Variable[42]; + U Another; + + void bar() {} + }; + + template<typename T> + struct Foo<T, bool> { + T Variable; + void bar() { ++Variable; } + }; + + template<> + struct Foo<unsigned, bool> { + unsigned [[Var^iable]]; + void bar() { ++[[Var^iable]]; } + }; + + void foo() { + Foo<unsigned, bool> f; + f.[[Var^iable]] = 9000; + } + )cpp", }; llvm::StringRef NewName = "NewName"; for (llvm::StringRef T : Tests) { Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -18,12 +18,12 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/SourceLocation.h" -#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/None.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" #include <algorithm> @@ -76,6 +76,8 @@ return OtherFile; } +const NamedDecl *canonicalRenameDecl(const NamedDecl *D); + llvm::DenseSet<const NamedDecl *> locateDeclAt(ParsedAST &AST, SourceLocation TokenStartLoc) { unsigned Offset = @@ -92,8 +94,7 @@ targetDecl(SelectedNode->ASTNode, DeclRelation::Alias | DeclRelation::TemplatePattern)) { // Get to CXXRecordDecl from constructor or destructor. - D = tooling::getCanonicalSymbolDeclaration(D); - Result.insert(D); + Result.insert(canonicalRenameDecl(D)); } return Result; } @@ -222,23 +223,89 @@ return error("Cannot rename symbol: {0}", Message(Reason)); } +const NamedDecl *canonicalRenameDecl(const ClassTemplateSpecializationDecl *D) { + return D->getSpecializedTemplate()->getTemplatedDecl(); +} + +const NamedDecl *canonicalRenameDecl(const TemplateDecl *D) { + return D->getTemplatedDecl(); +} + +const NamedDecl *canonicalRenameDecl(const CXXMethodDecl *D) { + const auto *Result = D; + if (const auto *InstantiatedMethod = D->getInstantiatedFromMemberFunction()) + Result = cast<CXXMethodDecl>(InstantiatedMethod); + while (Result->isVirtual() && Result->size_overridden_methods()) + Result = *Result->overridden_methods().begin(); + return Result; +} + +const NamedDecl *canonicalRenameDecl(const FunctionDecl *D) { + const auto *Definition = D->getDefinition(); + const auto *Candidate = Definition ? Definition : D->getMostRecentDecl(); + return Candidate->isTemplateInstantiation() + ? Candidate->getPrimaryTemplate()->getTemplatedDecl() + : Candidate; +} + +const NamedDecl *canonicalRenameDecl(const FieldDecl *D) { + // This is a hacky way to do something like + // CXXMethodDecl::getINstantiatedFromMemberFunction for the field because + // Clang AST does not store relevant information about the field that is + // instantiated. + const auto *TemplateSpec = + dyn_cast<ClassTemplateSpecializationDecl>(D->getParent()); + if (!TemplateSpec) + return D; + const auto *FieldParent = TemplateSpec->getTemplateInstantiationPattern(); + if (!FieldParent || D->getParent() == FieldParent) + return D; + for (const auto *Field : FieldParent->fields()) { + if (Field->getFieldIndex() == D->getFieldIndex()) { + assert(Field->getLocation() == D->getLocation() && + "D should be generated from Field so it has the same location."); + return Field; + } + } + llvm_unreachable("FieldParent should have field with same index as D."); +} + +// Canonical declarations help simplify the process of renaming. Examples: +// - Given a constructor/destructor, canonical declaration is the parent +// CXXRecordDecl +// - Specializations should point to the specialized declaration. +// - Instantiations should point to instantiated declaration. +// Some cases require recursive canonicalization: e.g. constructor -> parent +// class instantiation -> instantiated class. +const NamedDecl *canonicalRenameDecl(const NamedDecl *D) { + const auto *Candidate = D; + if (const auto *RD = dyn_cast<RecordDecl>(Candidate)) { + const auto *Definition = RD->getDefinition(); + Candidate = Definition ? Definition : RD->getMostRecentDecl(); + } + if (const auto *Constructor = dyn_cast<CXXConstructorDecl>(Candidate)) + return canonicalRenameDecl(Constructor->getParent()); + if (const auto *Destructor = dyn_cast<CXXDestructorDecl>(Candidate)) + return canonicalRenameDecl(Destructor->getParent()); + if (const auto *Template = dyn_cast<TemplateDecl>(Candidate)) + return canonicalRenameDecl(Template); + if (const auto *ClassTemplateSpecialization = + dyn_cast<ClassTemplateSpecializationDecl>(Candidate)) + return canonicalRenameDecl(ClassTemplateSpecialization); + if (const auto *Method = dyn_cast<CXXMethodDecl>(Candidate)) + return canonicalRenameDecl(Method); + if (const auto *Function = dyn_cast<FunctionDecl>(Candidate)) + return canonicalRenameDecl(Function); + if (const auto *Field = dyn_cast<FieldDecl>(Candidate)) + return canonicalRenameDecl(Field); + return Candidate; +} + // Return all rename occurrences in the main file. std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl &ND) { trace::Span Tracer("FindOccurrencesWithinFile"); - // If the cursor is at the underlying CXXRecordDecl of the - // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to - // get the primary template manually. - // getUSRsForDeclaration will find other related symbols, e.g. virtual and its - // overriddens, primary template and all explicit specializations. - // FIXME: Get rid of the remaining tooling APIs. - const auto *RenameDecl = - ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND; - std::vector<std::string> RenameUSRs = - tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext()); - llvm::DenseSet<SymbolID> TargetIDs; - for (auto &USR : RenameUSRs) - TargetIDs.insert(SymbolID(USR)); + const auto *RenameDecl = canonicalRenameDecl(&ND); std::vector<SourceLocation> Results; for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) { @@ -246,11 +313,11 @@ if (Ref.Targets.empty()) return; for (const auto *Target : Ref.Targets) { - auto ID = getSymbolID(Target); - if (!ID || TargetIDs.find(ID) == TargetIDs.end()) + if (canonicalRenameDecl(Target) == RenameDecl) { + Results.push_back(Ref.NameLoc); return; + } } - Results.push_back(Ref.NameLoc); }); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits