[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

The fact `TraverseCXXOperatorCallExpr` can't call its super `TraverseCXXOperatorCallExpr` makes the current solution kind of broken. The super implementation of `TraverseCXXOperatorCallExpr` is responsible for dispatch to WalkUp##STMT functions which actually call `VisitCXXOperatorCallExpr` which our current visitor will never call because of the custom "override". I would like to suggest an alternative approach that I think solves the problem more "elegantly":

Modify the `DEF_TRAVERSE_STMT` macro in RecursiveASTVisitor.h call to a wrapper function to get the children:

  class RecursiveASTVisitor {
  public:
  
    Stmt::child_range getStmtChildren(Stmt *S) {
      return S->children();
    }
   
  ...
  
  #define DEF_TRAVERSE_STMT(STMT, CODE)                                          \
    template <typename Derived>                                                  \
    bool RecursiveASTVisitor<Derived>::Traverse##STMT(                           \
        STMT *S, DataRecursionQueue *Queue) {                                    \
      bool ShouldVisitChildren = true;                                           \
      bool ReturnValue = true;                                                   \
      if (!getDerived().shouldTraversePostOrder())                               \
        TRY_TO(WalkUpFrom##STMT(S));                                             \
      { CODE; }                                                                  \
      if (ShouldVisitChildren) {                                                 \
        for (Stmt *SubStmt : getDerived().getStmtChildren(S)) {                                    \   // The only change is on this line.
          TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt);                              \
        }                                                                        \
      }                                                                          \
      if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder())       \
        TRY_TO(WalkUpFrom##STMT(S));                                             \
      return ReturnValue;                                                        \
    }
  
  ...
  
  };

Then you could provide an "override" in the `LexicallyOrderedRecursiveASTVisitor` that adjusts children just for `CXXOperatorCallExpr`. Something like this should work:

  class LexicallyOrderedRecursiveASTVisitor {
  public:
  ...
  
    Stmt::child_range getStmtChildren(Stmt *S) {
      return S->children();
    }
  
    SmallVector<Stmt *, 8> getStmtChildren(CXXOperatorCallExpr *CE) {
      SmallVector<Stmt *, 8> Children(CE->children());
      bool Swap;
      // Switch the operator and the first operand for all infix and postfix
      // operations.
      switch (CE->getOperator()) {
      case OO_Arrow:
      case OO_Call:
      case OO_Subscript:
        Swap = true;
        break;
      case OO_PlusPlus:
      case OO_MinusMinus:
        // These are postfix unless there is exactly one argument.
        Swap = Children.size() != 2;
        break;
      default:
        Swap = CE->isInfixBinaryOp();
        break;
      }
      if (Swap && Children.size() > 1)
        std::swap(Children[0], Children[1]);
      return Children;
    }
  
  ...
  };

WDYT?

Sorry about not seeing this earlier. Thanks for your patience!


https://reviews.llvm.org/D37200