Skip to content

Commit

Permalink
NameRefactorer:
Browse files Browse the repository at this point in the history 
	Replaced 'rCopy()' with 'std::reverse_copy()' to avoid
	unnecessary code.
Refactorer:
	Simplified output when printing a replacement.
FunctionRefactorer:
	Fixed an issue when refactoring methods in class hierarchies
	with multiple levels of inheritance.
  • Loading branch information
@stnuessl
stnuessl committed Jan 15, 2017
1 parent c4c8aa3 commit b65c509
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 43 deletions.
76 changes: 51 additions & 25 deletions src/Refactorers/FunctionRefactorer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@

#include <util/CommandLine.hpp>

static bool overrides(const clang::CXXMethodDecl *Decl)
{
return !!Decl->size_overridden_methods();
}

static bool overrides(const clang::FunctionDecl *Decl)
{
auto MethodDecl = clang::dyn_cast<clang::CXXMethodDecl>(Decl);
return (MethodDecl) ? overrides(MethodDecl) : false;
}

void FunctionRefactorer::visitCallExpr(const clang::CallExpr *Expr)
{
/*
Expand All @@ -33,7 +44,7 @@ void FunctionRefactorer::visitCallExpr(const clang::CallExpr *Expr)
if (!FuncDecl)
return;

if (isVictim(FuncDecl) || overridesVictim(FuncDecl))
if (isVictim(FuncDecl))
addReplacement(Expr->getCallee()->getExprLoc());
}

Expand All @@ -54,22 +65,28 @@ void FunctionRefactorer::visitDeclRefExpr(const clang::DeclRefExpr *Expr)
if (!FuncDecl)
return;

if (isVictim(FuncDecl) || overridesVictim(FuncDecl))
if (isVictim(FuncDecl))
addReplacement(Expr->getNameInfo().getLoc());
}

void FunctionRefactorer::visitFunctionDecl(const clang::FunctionDecl *Decl)
{
if (isVictim(Decl) || overridesVictim(Decl))
if (isVictim(Decl))
addReplacement(Decl->getLocation());
}

bool FunctionRefactorer::isVictim(const clang::FunctionDecl *Decl)
{
if (!NameRefactorer::isVictim(Decl))
return false;
/*
* We need to roll our own 'isVictim()' function here as we need to
* automatically detect derived and overridden methods to change
* them accordingly. Also, we want to avoid refactoring if the current
* function is a victim and overrides another function as this may alter
* the behaviour of the program.
*/

if (overrides(Decl) && !_Force) {
bool isVictimDecl = NameRefactorer::isVictim(Decl);
if (isVictimDecl && overrides(Decl) && !_Force) {
auto MethodDecl = clang::dyn_cast<clang::CXXMethodDecl>(Decl);

auto Begin = MethodDecl->begin_overridden_methods();
Expand All @@ -85,20 +102,41 @@ bool FunctionRefactorer::isVictim(const clang::FunctionDecl *Decl)
std::exit(EXIT_FAILURE);
}

return true;
/*
* Basically, if '_Force' is true the user can refactor methods which
* override another function. In this case we don't want to refactor
* methods further down the class hierarchy.
*/
return isVictimDecl || (!_Force && overridesVictim(Decl));
}

bool FunctionRefactorer::overridesVictim(const clang::CXXMethodDecl *Decl)
{
/*
* Check if 'Decl' is the victim. If not walk up the (possible) class
* hierarchy and check each overridden method if it is the victim.
* This is needed to ensure that all cases like in the following
* example are detected properly.
*
* class a { virtual void run() {} };
* ^(1)
* class b : public a { virtual void run() override {} };
* ^(2)
* class c : public b { virtual void run() override {} };
* ^(3)
* ...
*
* Command: $ rf --function a::run=process
*/

auto Begin = Decl->begin_overridden_methods();
auto End = Decl->end_overridden_methods();

const auto isVictimFunc = [this](const clang::CXXMethodDecl *Decl) {
return this->isVictim(Decl) || this->overridesVictim(Decl);
};

for (auto It = Begin; It != End; ++It) {
if (isVictim(*It))
return true;
}

return false;
return std::any_of(Begin, End, isVictimFunc);
}

bool FunctionRefactorer::overridesVictim(const clang::FunctionDecl *Decl)
Expand All @@ -107,15 +145,3 @@ bool FunctionRefactorer::overridesVictim(const clang::FunctionDecl *Decl)
return (MethodDecl) ? overridesVictim(MethodDecl) : false;
}


bool FunctionRefactorer::overrides(const clang::CXXMethodDecl *Decl)
{
return !!Decl->size_overridden_methods();
}

bool FunctionRefactorer::overrides(const clang::FunctionDecl *Decl)
{
auto MethodDecl = clang::dyn_cast<clang::CXXMethodDecl>(Decl);
return (MethodDecl) ? overrides(MethodDecl) : false;
}

4 changes: 1 addition & 3 deletions src/Refactorers/FunctionRefactorer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,11 @@ class FunctionRefactorer : public NameRefactorer {
virtual void visitCallExpr(const clang::CallExpr *Expr) override;
virtual void visitDeclRefExpr(const clang::DeclRefExpr *Expr) override;
virtual void visitFunctionDecl(const clang::FunctionDecl *Decl) override;

private:
bool isVictim(const clang::FunctionDecl *Decl);
bool overridesVictim(const clang::CXXMethodDecl *Decl);
bool overridesVictim(const clang::FunctionDecl *Decl);

static bool overrides(const clang::CXXMethodDecl *Decl);
static bool overrides(const clang::FunctionDecl *Decl);
};


Expand Down
23 changes: 9 additions & 14 deletions src/Refactorers/NameRefactorer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,6 @@ static bool isValidQualifierEnd(Iter Begin, Iter End)
return false;
}

static void rCopy(std::string &Str, const clang::StringRef &Ref)
{
auto n = Ref.size();

while (n--)
Str += Ref[n];
}

NameRefactorer::NameRefactorer()
: Refactorer(),
_Victim(),
Expand Down Expand Up @@ -351,10 +343,11 @@ bool NameRefactorer::isVictimLocation(const clang::SourceLocation Loc)
if (_VictimLoc.isValid())
return _VictimLoc == Loc;

auto &SM = _CompilerInstance->getSourceManager();
const auto &SM = _CompilerInstance->getSourceManager();
auto FullLoc = clang::FullSourceLoc(Loc, SM);
bool Invalid;

auto Line = SM.getSpellingLineNumber(Loc, &Invalid);
auto Line = FullLoc.getSpellingLineNumber(&Invalid);
if (Invalid) {
llvm::errs() << cl::Error()
<< "failed to retrieve line number for declaration \""
Expand All @@ -368,7 +361,7 @@ bool NameRefactorer::isVictimLocation(const clang::SourceLocation Loc)
if (!_Column)
return true;

auto Column = SM.getSpellingColumnNumber(Loc, &Invalid);
auto Column = FullLoc.getSpellingColumnNumber(&Invalid);
if (Invalid) {
llvm::errs() << cl::Error()
<< "failed to retrieve column number for declaration \""
Expand All @@ -394,16 +387,18 @@ NameRefactorer::qualifiedName(const clang::NamedDecl *NamedDecl)
*/

_Buffer.clear();
auto dest = std::back_inserter(_Buffer);
auto Name = NamedDecl->getName();

rCopy(_Buffer, NamedDecl->getName());
std::reverse_copy(Name.begin(), Name.end(), dest);
_Buffer += "::";

auto Context = NamedDecl->getDeclContext();

while (Context) {
NamedDecl = clang::dyn_cast<clang::NamedDecl>(Context);
if (NamedDecl) {
rCopy(_Buffer, NamedDecl->getName());
Name = NamedDecl->getName();
std::reverse_copy(Name.begin(), Name.end(), dest);
_Buffer += "::";
}

Expand Down
2 changes: 1 addition & 1 deletion src/Refactorers/Refactorer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,6 @@ void Refactorer::addReplacement(const clang::SourceManager &SM,
auto Ok = _ReplSet->insert(std::move(Repl)).second;
if (_Verbose && Ok) {
Loc.dump(SM);
llvm::errs() << " --> \"" << ReplText << "\"\n";
llvm::errs() << " -- \"" << ReplText << "\"\n";
}
}

0 comments on commit b65c509

Please sign in to comment.