<div class="gmail_extra">Thanks for letting me know about this. This change was falling foul of two g++ bugs:</div><div class="gmail_extra"> * Prior to g++ 4.3, it forgot to perform implicit conversions on pointer-to-member types</div>
<div class="gmail_extra"> * Prior to g++ 4.5/4.6, it didn't correctly handle injected-class-names from base classes</div><div class="gmail_extra"><br></div><div class="gmail_extra">I worked around these in r155969, and I think old versions of g++ should be happy now.</div>
<div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 1, 2012 at 4:18 PM, NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Richard, it seems it would be incompatible to g++...<br>
<br>
/home/bb/buildslave/cmake-clang-x86_64-linux/llvm-project/clang/unittests/Tooling/RecursiveASTVisitorTest.cpp:<br>
In member function ‘bool<br>
clang::CXXOperatorCallExprTraverser::TraverseCXXOperatorCallExpr(clang::CXXOperatorCallExpr*)’:<br>
/home/bb/buildslave/cmake-clang-x86_64-linux/llvm-project/clang/unittests/Tooling/RecursiveASTVisitorTest.cpp:175:<br>
error: ‘template<class T> class clang::ExpectedLocationVisitor’ used<br>
without template parameters<br>
<br>
$ g++44 --version<br>
g++44 (GCC) 4.4.6 20110731 (Red Hat 4.4.6-3)<br>
<br>
2012/5/2 Richard Smith <<a href="mailto:richard-llvm@metafoo.co.uk" target="_blank">richard-llvm@metafoo.co.uk</a>>:<br>
<div><div>> Author: rsmith<br>
> Date: Tue May 1 16:58:31 2012<br>
> New Revision: 155951<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=155951&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=155951&view=rev</a><br>
> Log:<br>
> Fix RecursiveASTVisitor's data recursion to call the Traverse* functions if they<br>
> have been overridden in the derived class. Also, remove a non-functional<br>
> implementation of an incorrect optimization for ParenExprs.<br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/AST/RecursiveASTVisitor.h<br>
> cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=155951&r1=155950&r2=155951&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=155951&r1=155950&r2=155951&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)<br>
> +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue May 1 16:58:31 2012<br>
> @@ -405,18 +405,14 @@<br>
> bool TraverseFunctionHelper(FunctionDecl *D);<br>
> bool TraverseVarHelper(VarDecl *D);<br>
><br>
> - bool Walk(Stmt *S);<br>
> -<br>
> struct EnqueueJob {<br>
> Stmt *S;<br>
> Stmt::child_iterator StmtIt;<br>
><br>
> - EnqueueJob(Stmt *S) : S(S), StmtIt() {<br>
> - if (Expr *E = dyn_cast_or_null<Expr>(S))<br>
> - S = E->IgnoreParens();<br>
> - }<br>
> + EnqueueJob(Stmt *S) : S(S), StmtIt() {}<br>
> };<br>
> bool dataTraverse(Stmt *S);<br>
> + bool dataTraverseNode(Stmt *S, bool &EnqueueChildren);<br>
> };<br>
><br>
> template<typename Derived><br>
> @@ -435,7 +431,12 @@<br>
><br>
> if (getDerived().shouldUseDataRecursionFor(CurrS)) {<br>
> if (job.StmtIt == Stmt::child_iterator()) {<br>
> - if (!Walk(CurrS)) return false;<br>
> + bool EnqueueChildren = true;<br>
> + if (!dataTraverseNode(CurrS, EnqueueChildren)) return false;<br>
> + if (!EnqueueChildren) {<br>
> + Queue.pop_back();<br>
> + continue;<br>
> + }<br>
> job.StmtIt = CurrS->child_begin();<br>
> } else {<br>
> ++job.StmtIt;<br>
> @@ -456,10 +457,16 @@<br>
> }<br>
><br>
> template<typename Derived><br>
> -bool RecursiveASTVisitor<Derived>::Walk(Stmt *S) {<br>
> +bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,<br>
> + bool &EnqueueChildren) {<br>
><br>
> + // Dispatch to the corresponding WalkUpFrom* function only if the derived<br>
> + // class didn't override Traverse* (and thus the traversal is trivial).<br>
> #define DISPATCH_WALK(NAME, CLASS, VAR) \<br>
> - return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR));<br>
> + if (&RecursiveASTVisitor::Traverse##NAME == &Derived::Traverse##NAME) \<br>
> + return getDerived().WalkUpFrom##NAME(static_cast<CLASS*>(VAR)); \<br>
> + EnqueueChildren = false; \<br>
> + return getDerived().Traverse##NAME(static_cast<CLASS*>(VAR));<br>
><br>
> if (BinaryOperator *BinOp = dyn_cast<BinaryOperator>(S)) {<br>
> switch (BinOp->getOpcode()) {<br>
><br>
> Modified: cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp?rev=155951&r1=155950&r2=155951&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp?rev=155951&r1=155950&r2=155951&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp (original)<br>
> +++ cfe/trunk/unittests/Tooling/RecursiveASTVisitorTest.cpp Tue May 1 16:58:31 2012<br>
> @@ -165,6 +165,25 @@<br>
> }<br>
> };<br>
><br>
> +class CXXOperatorCallExprTraverser<br>
> + : public ExpectedLocationVisitor<CXXOperatorCallExprTraverser> {<br>
> +public:<br>
> + // Use Traverse, not Visit, to check that data recursion optimization isn't<br>
> + // bypassing the call of this function.<br>
> + bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *CE) {<br>
> + Match(getOperatorSpelling(CE->getOperator()), CE->getExprLoc());<br>
> + return ExpectedLocationVisitor::TraverseCXXOperatorCallExpr(CE);<br>
> + }<br>
> +};<br>
> +<br>
> +class ParenExprVisitor : public ExpectedLocationVisitor<ParenExprVisitor> {<br>
> +public:<br>
> + bool VisitParenExpr(ParenExpr *Parens) {<br>
> + Match("", Parens->getExprLoc());<br>
> + return true;<br>
> + }<br>
> +};<br>
> +<br>
> TEST(RecursiveASTVisitor, VisitsBaseClassDeclarations) {<br>
> TypeLocVisitor Visitor;<br>
> Visitor.ExpectMatch("class X", 1, 30);<br>
> @@ -345,4 +364,20 @@<br>
> "vector_iterator<int> it_int;\n"));<br>
> }<br>
><br>
> +TEST(RecursiveASTVisitor, TraversesOverloadedOperator) {<br>
> + CXXOperatorCallExprTraverser Visitor;<br>
> + Visitor.ExpectMatch("()", 4, 9);<br>
> + EXPECT_TRUE(Visitor.runOver(<br>
> + "struct A {\n"<br>
> + " int operator()();\n"<br>
> + "} a;\n"<br>
> + "int k = a();\n"));<br>
> +}<br>
> +<br>
> +TEST(RecursiveASTVisitor, VisitsParensDuringDataRecursion) {<br>
> + ParenExprVisitor Visitor;<br>
> + Visitor.ExpectMatch("", 1, 9);<br>
> + EXPECT_TRUE(Visitor.runOver("int k = (4) + 9;\n"));<br>
> +}<br>
> +<br>
> } // end namespace clang<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>