mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

simplify postfixincrementfix plugin using parentStmt()

Browse Source 
Change-Id: I93fa422afe7f3e1e10576dd64af9d57b2302f44e
This commit is contained in:
Luboš Luňák 2013-06-20 00:31:37 +02:00
parent ade47d3d67
commit 81b58bb075
2 changed files with 27 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
compilerplugins/clang/postfixincrementfix.cxx
View File

@ -29,40 +29,20 @@ void PostfixIncrementFix::run()
TraverseDecl( compiler.getASTContext().getTranslationUnitDecl()); TraverseDecl( compiler.getASTContext().getTranslationUnitDecl());
} }
bool PostfixIncrementFix::VisitFunctionDecl( const FunctionDecl* declaration ) bool PostfixIncrementFix::VisitCXXOperatorCallExpr( const CXXOperatorCallExpr* op )
{ {
if( ignoreLocation( declaration )) if( ignoreLocation( op ))
return true; return true;
if( !declaration->doesThisDeclarationHaveABody()) // postfix ++ has two arguments (the operand and the hidden extra int)
return true; if( op->getOperator() == OO_PlusPlus && op->getNumArgs() == 2 )
StmtParents parents; fixPostfixOperator( op );
fixPostfixOperators( declaration->getBody(), parents ); // For primitive types it would be UnaryOperatorExpr, but probably no good reason to change those.
return true; return true;
} }
void PostfixIncrementFix::fixPostfixOperators( const Stmt* stmt, StmtParents& parents ) void PostfixIncrementFix::fixPostfixOperator( const CXXOperatorCallExpr* op )
{ {
if( const CXXOperatorCallExpr* op = dyn_cast<CXXOperatorCallExpr>( stmt )) if( !canChangePostfixToPrefix( op, op ))
{ // postfix ++ has two arguments (the operand and the hidden extra int)
if( op->getOperator() == OO_PlusPlus && op->getNumArgs() == 2 )
fixPostfixOperator( op, parents );
}
// For primitive types it would be UnaryOperatorExpr, but probably no good reason to change those.
parents.push_back( stmt );
for( ConstStmtIterator it = stmt->child_begin();
it != stmt->child_end();
++it )
{
if( *it != NULL ) // some children can be apparently NULL
fixPostfixOperators( *it, parents );
}
assert( parents.back() == stmt );
parents.pop_back();
}
void PostfixIncrementFix::fixPostfixOperator( const CXXOperatorCallExpr* op, StmtParents& parents )
{
if( !canChangePostfixToPrefix( op, parents, parents.size() - 1 ))
return; return;
if( !shouldDoChange( op->getArg( 0 ))) if( !shouldDoChange( op->getArg( 0 )))
return; return;
@ -73,10 +53,13 @@ void PostfixIncrementFix::fixPostfixOperator( const CXXOperatorCallExpr* op, Stm
removeText( op->getCallee()->getSourceRange()); removeText( op->getCallee()->getSourceRange());
} }
bool PostfixIncrementFix::canChangePostfixToPrefix( const CXXOperatorCallExpr* op, StmtParents& parents, int parent_pos ) bool PostfixIncrementFix::canChangePostfixToPrefix( const Stmt* stmt , const CXXOperatorCallExpr* op )
{ {
const Stmt* parent = parentStmt( stmt );
if( parent == NULL )
return true;
// check if foo++ can be safely replaced by ++foo // check if foo++ can be safely replaced by ++foo
switch( parents[ parent_pos ]->getStmtClass()) switch( parent->getStmtClass())
{ {
case Stmt::CompoundStmtClass: case Stmt::CompoundStmtClass:
return true; return true;
@ -91,52 +74,32 @@ bool PostfixIncrementFix::canChangePostfixToPrefix( const CXXOperatorCallExpr* o
case Stmt::CXXBindTemporaryExprClass: case Stmt::CXXBindTemporaryExprClass:
// tricky, it may just mean the temporary will be cleaned up // tricky, it may just mean the temporary will be cleaned up
// (by ExprWithCleanups), ignore and go up // (by ExprWithCleanups), ignore and go up
assert( parent_pos > 0 ); // should not happen return canChangePostfixToPrefix( parent, op );
return canChangePostfixToPrefix( op, parents, parent_pos - 1 );
case Stmt::ExprWithCleanupsClass: case Stmt::ExprWithCleanupsClass:
// cleanup of a temporary, should be harmless (if the use // cleanup of a temporary, should be harmless (if the use
// of the postfix ++ operator here relies on the fact that // of the postfix ++ operator here relies on the fact that
// the dtor for the object will be called, that's pretty insane // the dtor for the object will be called, that's pretty insane
// code). Ignore and go up. // code). Ignore and go up.
assert( parent_pos > 0 ); // should not happen return canChangePostfixToPrefix( parent, op );
return canChangePostfixToPrefix( op, parents, parent_pos - 1 );
case Stmt::ParenExprClass: // parentheses, go up case Stmt::ParenExprClass: // parentheses, go up
assert( parent_pos > 0 ); return canChangePostfixToPrefix( parent, op );
return canChangePostfixToPrefix( op, parents, parent_pos - 1 );
case Stmt::IfStmtClass: case Stmt::IfStmtClass:
return canChangeInConditionStatement( op, cast< IfStmt >( parents[ parent_pos ] )->getCond(), // cannot be changed in condition, can be changed in statements
parents, parent_pos ); return cast< IfStmt >( parent )->getCond() != stmt;
case Stmt::WhileStmtClass: case Stmt::WhileStmtClass:
return canChangeInConditionStatement( op, cast< WhileStmt >( parents[ parent_pos ] )->getCond(), return cast< WhileStmt >( parent )->getCond() != stmt;
parents, parent_pos );
case Stmt::DoStmtClass: case Stmt::DoStmtClass:
return canChangeInConditionStatement( op, cast< DoStmt >( parents[ parent_pos ] )->getCond(), return cast< DoStmt >( parent )->getCond() != stmt;
parents, parent_pos );
case Stmt::ForStmtClass: case Stmt::ForStmtClass:
return canChangeInConditionStatement( op, dyn_cast< ForStmt >( parents[ parent_pos ] )->getCond(), return cast< ForStmt >( parent )->getCond() != stmt;
parents, parent_pos );
default: default:
report( DiagnosticsEngine::Fatal, "cannot analyze operator++ (plugin needs fixing)", report( DiagnosticsEngine::Fatal, "cannot analyze operator++ (plugin needs fixing)",
op->getLocStart()) << parents[ parent_pos ]->getSourceRange(); op->getLocStart()) << parent->getSourceRange();
// parents[ parent_pos ]->dump(); parent->dump();
// parents[ std::max( parent_pos - 3, 0 ) ]->dump();
return false; return false;
} }
} }
bool PostfixIncrementFix::canChangeInConditionStatement( const CXXOperatorCallExpr* op, const Expr* condition,
const StmtParents& parents, unsigned int parent_pos )
{
// cannot be changed in condition, can be changed in statements
if( parent_pos == parents.size() - 1 )
return op != condition;
else
{ // indirect child
assert( parent_pos + 1 < parents.size());
return parents[ parent_pos + 1 ] != condition;
}
}
bool PostfixIncrementFix::shouldDoChange( const Expr* operand ) bool PostfixIncrementFix::shouldDoChange( const Expr* operand )
{ {
// TODO Changing 'a->b++' to '++a->b' is technically the same, but the latter probably looks confusing, // TODO Changing 'a->b++' to '++a->b' is technically the same, but the latter probably looks confusing,

11
compilerplugins/clang/postfixincrementfix.hxx
View File

@ -23,14 +23,11 @@ class PostfixIncrementFix
public: public:
explicit PostfixIncrementFix( CompilerInstance& compiler, Rewriter& rewriter ); explicit PostfixIncrementFix( CompilerInstance& compiler, Rewriter& rewriter );
virtual void run() override; virtual void run() override;
bool VisitFunctionDecl( const FunctionDecl* declaration ); bool VisitCXXOperatorCallExpr( const CXXOperatorCallExpr* op );
private: private:
typedef std::vector< const Stmt* > StmtParents; void fixPostfixOperator( const CXXOperatorCallExpr* op );
void fixPostfixOperator( const CXXOperatorCallExpr* op, StmtParents& parents ); void fixPostfixOperators( const Stmt* stmt );
void fixPostfixOperators( const Stmt* stmt, StmtParents& parents ); bool canChangePostfixToPrefix( const Stmt* stmt, const CXXOperatorCallExpr* op );
bool canChangePostfixToPrefix( const CXXOperatorCallExpr* op, StmtParents& parents, int parent_pos );
bool canChangeInConditionStatement( const CXXOperatorCallExpr* op, const Expr* condition,
const StmtParents& parents, unsigned int parent_pos );
bool shouldDoChange( const Expr* op ); bool shouldDoChange( const Expr* op );
}; };