Hi Daniel, Your change to always visit both syntactic and semantic forms of InitListExprs, by default, has downsides:
- It may be surprising and undesirable for some RecursiveASTVisitor clients that they see the same nodes visited twice - It can trigger exponential behavior, significantly degrading performance with huge initializer exprs; for example try visiting "llvm/src/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp” (and look into what 'lib/Target/X86/X86GenDisassemblerTables.inc’ contains). How about we make TraverseInitListExpr(InitListExpr *S) visit the semantic form only by default and add something like “visitationForInitLists()” that returns an enum for semantic/syntactic/both allowing RecursiveASTVisitor clients to control this ? > On Feb 4, 2015, at 6:29 AM, Daniel Jasper <[email protected]> wrote: > > Author: djasper > Date: Wed Feb 4 08:29:47 2015 > New Revision: 228144 > > URL: http://llvm.org/viewvc/llvm-project?rev=228144&view=rev > Log: > Rewrite r228138 to be somewhat saner. > > While probably technically correct, the solution r228138 was quite hard > to read/understand. This should be simpler. > > Also added a test to ensure that we are still visiting the syntactic form > as well. > > Modified: > cfe/trunk/include/clang/AST/RecursiveASTVisitor.h > cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp > > Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=228144&r1=228143&r2=228144&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original) > +++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Wed Feb 4 08:29:47 2015 > @@ -2032,20 +2032,21 @@ DEF_TRAVERSE_STMT(CXXStaticCastExpr, { > // to the syntactic form. > template <typename Derived> > bool RecursiveASTVisitor<Derived>::TraverseInitListExpr(InitListExpr *S) { > - if (InitListExpr *Syn = S->getSyntacticForm()) > - S = Syn; > - TRY_TO(WalkUpFromInitListExpr(S)); > - // All we need are the default actions. FIXME: use a helper function. > - for (Stmt::child_range range = S->children(); range; ++range) { > - TRY_TO(TraverseStmt(*range)); > - } > - if (InitListExpr *Syn = S->getSemanticForm()) { > + InitListExpr *Syn = S->isSemanticForm() ? S->getSyntacticForm() : S; > + if (Syn) { > TRY_TO(WalkUpFromInitListExpr(Syn)); > // All we need are the default actions. FIXME: use a helper function. > for (Stmt::child_range range = Syn->children(); range; ++range) { > TRY_TO(TraverseStmt(*range)); > } > } > + InitListExpr *Sem = S->isSemanticForm() ? S : S->getSemanticForm(); > + if (Sem) { > + TRY_TO(WalkUpFromInitListExpr(Sem)); > + for (Stmt::child_range range = Sem->children(); range; ++range) { > + TRY_TO(TraverseStmt(*range)); > + } > + } > return true; > } > > > Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=228144&r1=228143&r2=228144&view=diff > ============================================================================== > --- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original) > +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Wed Feb 4 08:29:47 > 2015 > @@ -3148,6 +3148,8 @@ TEST(InitListExpression, MatchesInitList > "void f();" > "S s[1] = { &f };", > declRefExpr(to(functionDecl(hasName("f")))))); > + EXPECT_TRUE( > + matches("int i[1] = {42, [0] = 43};", integerLiteral(equals(42)))); > } > > TEST(UsingDeclaration, MatchesUsingDeclarations) { > > > _______________________________________________ > 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
