simplify postfixincrementfix plugin using parentStmt()

Change-Id: I93fa422afe7f3e1e10576dd64af9d57b2302f44e

2 files changed, 27 insertions, 67 deletions

diff --git a/compilerplugins/clang/postfixincrementfix.cxx b/compilerplugins/clang/postfixincrementfix.cxx
index edb31e32e92c..ca636b9ffa10 100644
--- a/compilerplugins/clang/postfixincrementfix.cxx
+++ b/compilerplugins/clang/postfixincrementfix.cxx
@@ -29,40 +29,20 @@ void PostfixIncrementFix::run()
     TraverseDecl( compiler.getASTContext().getTranslationUnitDecl());
     }
 
-bool PostfixIncrementFix::VisitFunctionDecl( const FunctionDecl* declaration )
+bool PostfixIncrementFix::VisitCXXOperatorCallExpr( const CXXOperatorCallExpr* op )
     {
-    if( ignoreLocation( declaration ))
+    if( ignoreLocation( op ))
         return true;
-    if( !declaration->doesThisDeclarationHaveABody())
-        return true;
-    StmtParents parents;
-    fixPostfixOperators( declaration->getBody(), parents );
-    return true;
-    }
-
-void PostfixIncrementFix::fixPostfixOperators( const Stmt* stmt, StmtParents& parents )
-    {
-    if( const CXXOperatorCallExpr* op = dyn_cast<CXXOperatorCallExpr>( stmt ))
-        { // postfix ++ has two arguments (the operand and the hidden extra int)
-        if( op->getOperator() == OO_PlusPlus && op->getNumArgs() == 2 )
-            fixPostfixOperator( op, parents );
-        }
+    // postfix ++ has two arguments (the operand and the hidden extra int)
+    if( op->getOperator() == OO_PlusPlus && op->getNumArgs() == 2 )
+        fixPostfixOperator( op );
     // 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();
+    return true;
     }
 
-void PostfixIncrementFix::fixPostfixOperator( const CXXOperatorCallExpr* op, StmtParents& parents )
+void PostfixIncrementFix::fixPostfixOperator( const CXXOperatorCallExpr* op )
     {
-    if( !canChangePostfixToPrefix( op, parents, parents.size() - 1 ))
+    if( !canChangePostfixToPrefix( op, op ))
         return;
     if( !shouldDoChange( op->getArg( 0 )))
         return;
@@ -73,10 +53,13 @@ void PostfixIncrementFix::fixPostfixOperator( const CXXOperatorCallExpr* op, Stm
         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
-    switch( parents[ parent_pos ]->getStmtClass())
+    switch( parent->getStmtClass())
         {
         case Stmt::CompoundStmtClass:
             return true;
@@ -91,52 +74,32 @@ bool PostfixIncrementFix::canChangePostfixToPrefix( const CXXOperatorCallExpr* o
         case Stmt::CXXBindTemporaryExprClass:
             // tricky, it may just mean the temporary will be cleaned up
             // (by ExprWithCleanups), ignore and go up
-            assert( parent_pos > 0 ); // should not happen
-            return canChangePostfixToPrefix( op, parents, parent_pos - 1 );
+            return canChangePostfixToPrefix( parent, op );
         case Stmt::ExprWithCleanupsClass:
             // cleanup of a temporary, should be harmless (if the use
             // of the postfix ++ operator here relies on the fact that
             // the dtor for the object will be called, that's pretty insane
             // code). Ignore and go up.
-            assert( parent_pos > 0 ); // should not happen
-            return canChangePostfixToPrefix( op, parents, parent_pos - 1 );
+            return canChangePostfixToPrefix( parent, op );
         case Stmt::ParenExprClass: // parentheses, go up
-            assert( parent_pos > 0 );
-            return canChangePostfixToPrefix( op, parents, parent_pos - 1 );
+            return canChangePostfixToPrefix( parent, op );
         case Stmt::IfStmtClass:
-            return canChangeInConditionStatement( op, cast< IfStmt >( parents[ parent_pos ] )->getCond(),
-                parents, parent_pos );
+            // cannot be changed in condition, can be changed in statements
+            return cast< IfStmt >( parent )->getCond() != stmt;
         case Stmt::WhileStmtClass:
-            return canChangeInConditionStatement( op, cast< WhileStmt >( parents[ parent_pos ] )->getCond(),
-                parents, parent_pos );
+            return cast< WhileStmt >( parent )->getCond() != stmt;
         case Stmt::DoStmtClass:
-            return canChangeInConditionStatement( op, cast< DoStmt >( parents[ parent_pos ] )->getCond(),
-                parents, parent_pos );
+            return cast< DoStmt >( parent )->getCond() != stmt;
         case Stmt::ForStmtClass:
-            return canChangeInConditionStatement( op, dyn_cast< ForStmt >( parents[ parent_pos ] )->getCond(),
-                parents, parent_pos );
+            return cast< ForStmt >( parent )->getCond() != stmt;
         default:
             report( DiagnosticsEngine::Fatal, "cannot analyze operator++ (plugin needs fixing)",
-                op->getLocStart()) << parents[ parent_pos ]->getSourceRange();
-//            parents[ parent_pos ]->dump();
-//            parents[ std::max( parent_pos - 3, 0 ) ]->dump();
+                op->getLocStart()) << parent->getSourceRange();
+            parent->dump();
             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 )
     {
     // TODO Changing 'a->b++' to '++a->b' is technically the same, but the latter probably looks confusing,
diff --git a/compilerplugins/clang/postfixincrementfix.hxx b/compilerplugins/clang/postfixincrementfix.hxx
index 29756cf690c6..e357f9941caa 100644
--- a/compilerplugins/clang/postfixincrementfix.hxx
+++ b/compilerplugins/clang/postfixincrementfix.hxx
@@ -23,14 +23,11 @@ class PostfixIncrementFix
     public:
         explicit PostfixIncrementFix( CompilerInstance& compiler, Rewriter& rewriter );
         virtual void run() override;
-        bool VisitFunctionDecl( const FunctionDecl* declaration );
+        bool VisitCXXOperatorCallExpr( const CXXOperatorCallExpr* op );
     private:
-        typedef std::vector< const Stmt* > StmtParents;
-        void fixPostfixOperator( const CXXOperatorCallExpr* op, StmtParents& parents );
-        void fixPostfixOperators( const Stmt* stmt, StmtParents& parents );
-        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 );
+        void fixPostfixOperator( const CXXOperatorCallExpr* op );
+        void fixPostfixOperators( const Stmt* stmt );
+        bool canChangePostfixToPrefix( const Stmt* stmt, const CXXOperatorCallExpr* op );
         bool shouldDoChange( const Expr* op );
     };