VisitUnaryOperator called twice in RecursiveASTVisitor when post-order traversal

Hi,

I'm new to Clang LibTooling. I am using `RecursiveASTVisitor` to
traverse the AST in post-order (i.e. with `shouldTraversePostOrder()`
returning true). I am finding that one particular `Visit` function,
`VisitUnaryOperator` is called twice (other `Visit` functions seem to
be fine). In pre-order traversal, it is called once as expected. I'm
using the latest svn Clang. Am I missing something?

Here is minimal example which illustrates the problem (based on
[this]( Modern source-to-source transformation with Clang and libTooling - Eli Bendersky's website)
example)

    #include <iostream>

    #include "clang/AST/AST.h"
    #include "clang/AST/ASTConsumer.h"
    #include "clang/AST/RecursiveASTVisitor.h"
    #include "clang/Frontend/ASTConsumers.h"
    #include "clang/Frontend/CompilerInstance.h"
    #include "clang/Frontend/FrontendActions.h"
    #include "clang/Tooling/CommonOptionsParser.h"
    #include "clang/Tooling/Tooling.h"

    using namespace clang;
    using namespace clang::driver;
    using namespace clang::tooling;

    static llvm::cl::OptionCategory ToolingSampleCategory("Tooling Sample");

    class MyASTVisitor : public RecursiveASTVisitor<MyASTVisitor> {
    public:
      MyASTVisitor() {
      }

      bool shouldTraversePostOrder() const {
        // if returns false then "Inside VisitUnaryOperator" printed
once (expected)
        // if returns true then "Inside VisitUnaryOperator" printed
twice (why?)
        return true;
      }

      bool VisitUnaryOperator(UnaryOperator * s){
        std::cout << "Inside VisitUnaryOperator" << std::endl;
        return true;
      }

    };

    class MyASTConsumer : public ASTConsumer {
    public:
      MyASTConsumer() : Visitor() {}

      bool HandleTopLevelDecl(DeclGroupRef DR) override {
        for (DeclGroupRef::iterator b = DR.begin(), e = DR.end(); b != e; ++b) {
          Visitor.TraverseDecl(*b);
        }
        return true;
      }

    private:
      MyASTVisitor Visitor;
    };

    class MyFrontendAction : public ASTFrontendAction {
    public:
      MyFrontendAction() {}
      std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                     StringRef file) override {
        return llvm::make_unique<MyASTConsumer>();
      }

    };

    int main(int argc, const char **argv) {

      CommonOptionsParser op(argc, argv, ToolingSampleCategory);
      ClangTool Tool(op.getCompilations(), op.getSourcePathList());

      return Tool.run(newFrontendActionFactory<MyFrontendAction>().get());
    }

And here is the code snippet which this tool is run on:

    int main() {
      int x = -1;
      return 0;
    }

Does this fix it?

diff --git a/include/clang/AST/RecursiveASTVisitor.h
b/include/clang/AST/RecursiveASTVisitor.h
index 99f46c7..6b4d998d 100644
--- a/include/clang/AST/RecursiveASTVisitor.h
+++ b/include/clang/AST/RecursiveASTVisitor.h
@@ -357,7 +357,8 @@ public:
#define OPERATOR(NAME)
         \
   bool TraverseUnary##NAME(UnaryOperator *S,
         \
                            DataRecursionQueue *Queue = nullptr) {
         \
- TRY_TO(WalkUpFromUnary##NAME(S));
         \
+ if (!getDerived().shouldTraversePostOrder())
         \
+ TRY_TO(WalkUpFromUnary##NAME(S));
         \
     TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getSubExpr());
         \
     return true;
         \
   }
         \

Yes, much appreciated.