omtcyfz updated this revision to Diff 64285. omtcyfz marked an inline comment as done. omtcyfz added a comment.
replaced RecursiveASTVisitor with ASTMatcher https://reviews.llvm.org/D22408 Files: clang-rename/USRFindingAction.cpp test/clang-rename/VirtualFunctionFindInBaseClass.cpp test/clang-rename/VirtualFunctionFindInDerivedClass.cpp
Index: test/clang-rename/VirtualFunctionFindInDerivedClass.cpp =================================================================== --- /dev/null +++ test/clang-rename/VirtualFunctionFindInDerivedClass.cpp @@ -0,0 +1,33 @@ +// RUN: cat %s > %t.cpp +// RUN: clang-rename -offset=165 -new-name=bar %t.cpp -i -- +// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s + +class Foo { +public: + virtual void foo() {} // CHECK: virtual void bar() {} +}; + +class Bar : public Foo { +public: + void foo() override {} // CHECK: void bar() override {} +}; + +class Baz : public Bar { + public: + void foo() override {} // CHECK: void bar() override {} +}; + +int main() { + Foo f; + f.foo(); // CHECK: f.bar(); + + Bar b; + b.foo(); // CHECK: b.bar(); + + Baz obj; + obj.foo(); // CHECK: obj.bar(); + + return 0; +} + +// Use grep -FUbo 'foo' <file> to get the correct offset of foo when changing Index: test/clang-rename/VirtualFunctionFindInBaseClass.cpp =================================================================== --- /dev/null +++ test/clang-rename/VirtualFunctionFindInBaseClass.cpp @@ -0,0 +1,33 @@ +// RUN: cat %s > %t.cpp +// RUN: clang-rename -offset=165 -new-name=bar %t.cpp -i -- +// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s + +class Foo { +public: + virtual void foo() {} // CHECK: virtual void bar() {} +}; + +class Bar : public Foo { +public: + void foo() override {} // CHECK: void bar() override {} +}; + +class Baz : public Bar { + public: + void foo() override {} // CHECK: void bar() override {} +}; + +int main() { + Foo f; + f.foo(); // CHECK: f.bar(); + + Bar b; + b.foo(); // CHECK: b.bar(); + + Baz obj; + obj.foo(); // CHECK: obj.bar(); + + return 0; +} + +// Use grep -FUbo 'foo' <file> to get the correct offset of foo when changing Index: clang-rename/USRFindingAction.cpp =================================================================== --- clang-rename/USRFindingAction.cpp +++ clang-rename/USRFindingAction.cpp @@ -8,64 +8,124 @@ //===----------------------------------------------------------------------===// /// /// \file -/// \brief Provides an action to rename every symbol at a point. +/// \brief Provides an action to find USR for the symbol at <offset>, as well as +/// all relevant USRs. /// //===----------------------------------------------------------------------===// #include "USRFindingAction.h" #include "USRFinder.h" #include "clang/AST/AST.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/FileManager.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Tooling.h" +#include <algorithm> #include <string> +#include <set> #include <vector> + using namespace llvm; +using namespace clang::ast_matchers; namespace clang { namespace rename { -// Get the USRs for the constructors of the class. -static std::vector<std::string> getAllConstructorUSRs( - const CXXRecordDecl *Decl) { - std::vector<std::string> USRs; - - // We need to get the definition of the record (as opposed to any forward - // declarations) in order to find the constructor and destructor. - const auto *RecordDecl = Decl->getDefinition(); - - // Iterate over all the constructors and add their USRs. - for (const auto *CtorDecl : RecordDecl->ctors()) - USRs.push_back(getUSRForDecl(CtorDecl)); - - // Ignore destructors. GetLocationsOfUSR will find the declaration of and - // explicit calls to a destructor through TagTypeLoc (and it is better for the - // purpose of renaming). - // - // For example, in the following code segment, - // 1 class C { - // 2 ~C(); - // 3 }; - // At line 3, there is a NamedDecl starting from '~' and a TagTypeLoc starting - // from 'C'. - - return USRs; -} +namespace { +// \brief NamedDeclFindingConsumer should delegate finding USRs relevant to +// given Decl to RelevantUSRFinder. +// +// FIXME: It's better to match ctors/dtors via typeLoc's instead of adding +// their USRs to the storage, because we can also match CXXConversionDecl's by +// typeLoc and we won't have to "manually" handle them here. +class RelevantUSRFinder : public MatchFinder::MatchCallback { +public: + explicit RelevantUSRFinder(const Decl *FoundDecl, ASTContext &Context, + std::vector<std::string> *USRs) + : FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {} + + void Find() { + USRSet.insert(getUSRForDecl(FoundDecl)); + addRelevantUSRs(); + addMatchers(); + Finder.matchAST(Context); + USRs->insert(USRs->end(), USRSet.begin(), USRSet.end()); + } + +private: + void addMatchers() { + const auto CXXMethodDeclMatcher = + cxxMethodDecl(isVirtual()).bind("cxxMethodDecl"); + Finder.addMatcher(CXXMethodDeclMatcher, this); + } + + virtual void run(const MatchFinder::MatchResult &Result) { + const auto *VirtualMethod = + Result.Nodes.getNodeAs<CXXMethodDecl>("cxxMethodDecl"); + bool Found = false; + for (const auto &OverriddenMethod : VirtualMethod->overridden_methods()) { + if (USRSet.find(getUSRForDecl(OverriddenMethod)) != USRSet.end()) { + Found = true; + } + } + if (Found) { + USRSet.insert(getUSRForDecl(VirtualMethod)); + } + } + + void addRelevantUSRs() { + // If D is CXXRecordDecl we should add all USRs of its constructors. + if (const auto *RecordDecl = dyn_cast<CXXRecordDecl>(FoundDecl)) { + // Ignore destructors. Find the declaration of and explicit calls to a + // destructor through TagTypeLoc (and it is better for the purpose of + // renaming). + // + // For example, in the following code segment, + // 1 class C { + // 2 ~C(); + // 3 }; + // + // At line 3, there is a NamedDecl starting from '~' and a TagTypeLoc + // starting from 'C'. + RecordDecl = RecordDecl->getDefinition(); + + // Iterate over all the constructors and add their USRs. + for (const auto *CtorDecl : RecordDecl->ctors()) { + USRSet.insert(getUSRForDecl(CtorDecl)); + } + } + // If D is CXXMethodDecl we should add all USRs of its overriden methods. + if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FoundDecl)) { + for (auto &OverriddenMethod : MethodDecl->overridden_methods()) { + USRSet.insert(getUSRForDecl(OverriddenMethod)); + } + } + } + + const Decl *FoundDecl; + ASTContext &Context; + std::vector<std::string> *USRs; + std::set<std::string> USRSet; + MatchFinder Finder; +}; +} // namespace struct NamedDeclFindingConsumer : public ASTConsumer { void HandleTranslationUnit(ASTContext &Context) override { const auto &SourceMgr = Context.getSourceManager(); // The file we look for the USR in will always be the main source file. - const auto Point = SourceMgr.getLocForStartOfFile( - SourceMgr.getMainFileID()).getLocWithOffset(SymbolOffset); + const auto Point = SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID()) + .getLocWithOffset(SymbolOffset); if (!Point.isValid()) return; const NamedDecl *FoundDecl = nullptr; @@ -84,30 +144,26 @@ return; } - // If the decl is a constructor or destructor, we want to instead take the - // decl of the parent record. - if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FoundDecl)) + // If FoundDecl is a constructor or destructor, we want to instead take the + // Decl of the corresponding class. + if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FoundDecl)) { FoundDecl = CtorDecl->getParent(); - else if (const auto *DtorDecl = dyn_cast<CXXDestructorDecl>(FoundDecl)) + } else if (const auto *DtorDecl = dyn_cast<CXXDestructorDecl>(FoundDecl)) { FoundDecl = DtorDecl->getParent(); - - // If the decl is in any way relatedpp to a class, we want to make sure we - // search for the constructor and destructor as well as everything else. - if (const auto *Record = dyn_cast<CXXRecordDecl>(FoundDecl)) - *USRs = getAllConstructorUSRs(Record); - - USRs->push_back(getUSRForDecl(FoundDecl)); + } *SpellingName = FoundDecl->getNameAsString(); + + RelevantUSRFinder Finder(FoundDecl, Context, USRs); + Finder.Find(); } unsigned SymbolOffset; std::string OldName; std::string *SpellingName; std::vector<std::string> *USRs; }; -std::unique_ptr<ASTConsumer> -USRFindingAction::newASTConsumer() { +std::unique_ptr<ASTConsumer> USRFindingAction::newASTConsumer() { std::unique_ptr<NamedDeclFindingConsumer> Consumer( new NamedDeclFindingConsumer); SpellingName = "";
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits