[cfe-commits] r155951 - in /cfe/trunk: include/clang/AST/RecursiveASTVisitor.h unittests/Tooling/RecursiveASTVisitorTest.cpp

[Richard Smith]

Thanks for letting me know about this. This change was falling foul of two
g++ bugs:
 * Prior to g++ 4.3, it forgot to perform implicit conversions on
pointer-to-member types
 * Prior to g++ 4.5/4.6, it didn't correctly handle injected-class-names
from base classes

I worked around these in r155969, and I think old versions of g++ should be
happy now.

On Tue, May 1, 2012 at 4:18 PM, NAKAMURA Takumi <geek4civic at gmail.com>wrote:

> Richard, it seems it would be incompatible to g++...
>
>
> /home/bb/buildslave/cmake-clang-x86_64-linux/llvm-project/clang/unittests/Tooling/RecursiveASTVisitorTest.cpp:
> In member function ‘bool
>
> clang::CXXOperatorCallExprTraverser::TraverseCXXOperatorCallExpr(clang::CXXOperatorCallExpr*)’:
>
> /home/bb/buildslave/cmake-clang-x86_64-linux/llvm-project/clang/unittests/Tooling/RecursiveASTVisitorTest.cpp:175:
> error: ‘template<class T> class clang::ExpectedLocationVisitor’ used
> without template parameters
>
> $ g++44 --version
> g++44 (GCC) 4.4.6 20110731 (Red Hat 4.4.6-3)
>
> 2012/5/2 Richard Smith <richard-llvm at metafoo.co.uk>:
> > Author: rsmith
> > Date: Tue May  1 16:58:31 2012
> > New Revision: 155951
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=155951&view=rev
> > Log:
> > Fix RecursiveASTVisitor's data recursion to call the Traverse* functions
> if they
> > have been overridden in the derived class. Also, remove a non-functional
> > implementation of an incorrect optimization for ParenExprs.
> >
> > Modified:
> >    cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
> >    cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp
> >
> > Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=155951&r1=155950&r2=155951&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
> > +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue May  1
> 16:58:31 2012
> > @@ -405,18 +405,14 @@
> >   bool TraverseFunctionHelper(FunctionDecl *D);
> >   bool TraverseVarHelper(VarDecl *D);
> >
> > -  bool Walk(Stmt *S);
> > -
> >   struct EnqueueJob {
> >     Stmt *S;
> >     Stmt::child_iterator StmtIt;
> >
> > -    EnqueueJob(Stmt *S) : S(S), StmtIt() {
> > -      if (Expr *E = dyn_cast_or_null<Expr>(S))
> > -        S = E->IgnoreParens();
> > -    }
> > +    EnqueueJob(Stmt *S) : S(S), StmtIt() {}
> >   };
> >   bool dataTraverse(Stmt *S);
> > +  bool dataTraverseNode(Stmt *S, bool &EnqueueChildren);
> >  };
> >
> >  template<typename Derived>
> > @@ -435,7 +431,12 @@
> >
> >     if (getDerived().shouldUseDataRecursionFor(CurrS)) {
> >       if (job.StmtIt == Stmt::child_iterator()) {
> > -        if (!Walk(CurrS)) return false;
> > +        bool EnqueueChildren = true;
> > +        if (!dataTraverseNode(CurrS, EnqueueChildren)) return false;
> > +        if (!EnqueueChildren) {
> > +          Queue.pop_back();
> > +          continue;
> > +        }
> >         job.StmtIt = CurrS->child_begin();
> >       } else {
> >         ++job.StmtIt;
> > @@ -456,10 +457,16 @@
> >  }
> >
> >  template<typename Derived>
> > -bool RecursiveASTVisitor<Derived>::Walk(Stmt *S) {
> > +bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,
> > +                                                    bool
> &EnqueueChildren) {
> >
> > +  // Dispatch to the corresponding WalkUpFrom* function only if the
> derived
> > +  // class didn't override Traverse* (and thus the traversal is
> trivial).
> >  #define DISPATCH_WALK(NAME, CLASS, VAR) \
> > -  return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR));
> > +  if (&RecursiveASTVisitor::Traverse##NAME == &Derived::Traverse##NAME)
> \
> > +    return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR)); \
> > +  EnqueueChildren = false; \
> > +  return getDerived().Traverse##NAME(static_cast<CLASS*>(VAR));
> >
> >   if (BinaryOperator *BinOp = dyn_cast<BinaryOperator>(S)) {
> >     switch (BinOp->getOpcode()) {
> >
> > Modified: cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp?rev=155951&r1=155950&r2=155951&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp (original)
> > +++ cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp Tue May  1
> 16:58:31 2012
> > @@ -165,6 +165,25 @@
> >   }
> >  };
> >
> > +class CXXOperatorCallExprTraverser
> > +  : public ExpectedLocationVisitor<CXXOperatorCallExprTraverser> {
> > +public:
> > +  // Use Traverse, not Visit, to check that data recursion optimization
> isn't
> > +  // bypassing the call of this function.
> > +  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *CE) {
> > +    Match(getOperatorSpelling(CE->getOperator()), CE->getExprLoc());
> > +    return ExpectedLocationVisitor::TraverseCXXOperatorCallExpr(CE);
> > +  }
> > +};
> > +
> > +class ParenExprVisitor : public
> ExpectedLocationVisitor<ParenExprVisitor> {
> > +public:
> > +  bool VisitParenExpr(ParenExpr *Parens) {
> > +    Match("", Parens->getExprLoc());
> > +    return true;
> > +  }
> > +};
> > +
> >  TEST(RecursiveASTVisitor, VisitsBaseClassDeclarations) {
> >   TypeLocVisitor Visitor;
> >   Visitor.ExpectMatch("class X", 1, 30);
> > @@ -345,4 +364,20 @@
> >     "vector_iterator<int> it_int;\n"));
> >  }
> >
> > +TEST(RecursiveASTVisitor, TraversesOverloadedOperator) {
> > +  CXXOperatorCallExprTraverser Visitor;
> > +  Visitor.ExpectMatch("()", 4, 9);
> > +  EXPECT_TRUE(Visitor.runOver(
> > +    "struct A {\n"
> > +    "  int operator()();\n"
> > +    "} a;\n"
> > +    "int k = a();\n"));
> > +}
> > +
> > +TEST(RecursiveASTVisitor, VisitsParensDuringDataRecursion) {
> > +  ParenExprVisitor Visitor;
> > +  Visitor.ExpectMatch("", 1, 9);
> > +  EXPECT_TRUE(Visitor.runOver("int k = (4) + 9;\n"));
> > +}
> > +
> >  } // end namespace clang
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120501/661bf690/attachment.html>