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 <[email protected]>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 <[email protected]>: > > 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 > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
