} Why this change? This reverts part of the support I added Weird, I have no idea how that happened. It's fine by me to change it back. Feel free to do it yourself, or let me know if you'd like me to.
craig ________________________________ From: Nick Lewycky <[email protected]> To: Craig Silverstein <[email protected]> Cc: [email protected] Sent: Fri, July 30, 2010 7:05:04 PM Subject: Re: [cfe-commits] r109590 - /cfe/trunk/include/clang/AST/RecursiveASTVisitor.h On 28 July 2010 08:54, Craig Silverstein <[email protected]> wrote: Author: csilvers >Date: Wed Jul 28 10:54:33 2010 >New Revision: 109590 > >URL: http://llvm.org/viewvc/llvm-project?rev=109590&view=rev >Log: > Add proper callbacks for DeclStmt -- we weren't recursing on > the decls. This was just an oversight before; one we didn't > catch because lots of information in a DeclStmt was also being > traversed (redundantly) elsewhere. > > Once DeclStmt was cleaned up, I could clean up some of the > redundant traversals found elswhere as well -- in particular, > traversing the declarations inside a function as part of the > function callback (instead of as part of the CompoundExpr > callback that constitutes the body of the function). The old > way was really weird, and led to some parts of local variable > declarations (but not all) being visited twice. That is now > resolved. I also was able to simplify the traversers for > IfStmt/WhileStmt/etc, which used to have redundant calls to > work around the fact DeclStmt wasn't working properly. > > While in the area, I fixed up a few more recursion-ordering > issues. I try to hold to the principle that > RecursiveASTVisitor visits objects in the source code in the > same order they're typed. So the return-type of a variable > comes before the variable-name. This still isn't perfect, but > we're closer to that. > > Reviewed by chandlerc and wan. > >Modified: > cfe/trunk/include/clang/AST/RecursiveASTVisitor.h > >Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h >URL: >http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=109590&r1=109589&r2=109590&view=diff > > >============================================================================== >--- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original) >+++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Wed Jul 28 10:54:33 2010 >@@ -1119,10 +1119,10 @@ > > DEF_TRAVERSE_DECL(TemplateTypeParmDecl, { > // D is the "T" in something like "template<typename T> class vector;" >- if (D->hasDefaultArgument()) >- TRY_TO(TraverseTypeLoc(D->getDefaultArgumentInfo()->getTypeLoc())); > if (D->getTypeForDecl()) > TRY_TO(TraverseType(QualType(D->getTypeForDecl(), 0))); >+ if (D->hasDefaultArgument()) >+ TRY_TO(TraverseTypeLoc(D->getDefaultArgumentInfo()->getTypeLoc())); > }) > > DEF_TRAVERSE_DECL(TypedefDecl, { >@@ -1175,7 +1175,7 @@ > for (CXXRecordDecl::base_class_iterator I = D->bases_begin(), > E = D->bases_end(); > I != E; ++I) { >- TRY_TO(TraverseTypeLoc(I->getTypeSourceInfo()->getTypeLoc())); >+ TRY_TO(TraverseType(I->getType())); > Why this change? This reverts part of the support I added for getting source location information of base declarations. Nick } > // We don't traverse the friends or the conversions, as they are > // already in decls_begin()/decls_end(). >@@ -1312,6 +1312,8 @@ > return true; > } > >+ TRY_TO(TraverseType(D->getResultType())); >+ > // If we're an explicit template specialization, iterate over the > // template args that were explicitly specified. > if (const FunctionTemplateSpecializationInfo *FTSI = >@@ -1328,9 +1330,11 @@ > } > } > >- TRY_TO(TraverseType(D->getResultType())); >+ for (FunctionDecl::param_iterator I = D->param_begin(), E = D->param_end(); >+ I != E; ++I) { >+ TRY_TO(TraverseDecl(*I)); >+ } > >- // FIXME: put this after the function parameters but before the body. > if (FunctionProtoType *FuncProto = dyn_cast<FunctionProtoType>(FuncType)) { > if (D->isThisDeclarationADefinition()) { > // This would be visited if we called TraverseType(D->getType()) >@@ -1349,8 +1353,6 @@ > } > } > >- TRY_TO(TraverseDeclContextHelper(D)); // Parameters. >- > if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) { > // Constructor initializers. > for (CXXConstructorDecl::init_iterator I = Ctor->init_begin(), >@@ -1401,9 +1403,6 @@ > template<typename Derived> > bool RecursiveASTVisitor<Derived>::TraverseVarHelper(VarDecl *D) { > TRY_TO(TraverseDeclaratorHelper(D)); >- // FIXME: This often double-counts -- for instance, for all local >- // vars, though not for global vars -- because the initializer is >- // also captured when the var-decl is in a DeclStmt. > TRY_TO(TraverseStmt(D->getInit())); > return true; > } >@@ -1418,11 +1417,13 @@ > > DEF_TRAVERSE_DECL(NonTypeTemplateParmDecl, { > // A non-type template parameter, e.g. "S" in template<int S> class Foo ... >- TRY_TO(TraverseStmt(D->getDefaultArgument())); > TRY_TO(TraverseVarHelper(D)); >+ TRY_TO(TraverseStmt(D->getDefaultArgument())); > }) > > DEF_TRAVERSE_DECL(ParmVarDecl, { >+ TRY_TO(TraverseVarHelper(D)); >+ > if (D->hasDefaultArg() && > D->hasUninstantiatedDefaultArg() && > !D->hasUnparsedDefaultArg()) >@@ -1432,8 +1433,6 @@ > !D->hasUninstantiatedDefaultArg() && > !D->hasUnparsedDefaultArg()) > TRY_TO(TraverseStmt(D->getDefaultArg())); >- >- TRY_TO(TraverseVarHelper(D)); > }) > > #undef DEF_TRAVERSE_DECL >@@ -1476,35 +1475,36 @@ > }) > > DEF_TRAVERSE_STMT(CXXCatchStmt, { >- // We don't traverse S->getCaughtType(), as we are already >- // traversing the exception object, which has this type. >+ TRY_TO(TraverseDecl(S->getExceptionDecl())); > // child_begin()/end() iterates over the handler block. > }) > >-DEF_TRAVERSE_STMT(ForStmt, { >- TRY_TO(TraverseDecl(S->getConditionVariable())); >- // child_begin()/end() iterates over init, cond, inc, and body stmts. >- }) >- >-DEF_TRAVERSE_STMT(IfStmt, { >- TRY_TO(TraverseDecl(S->getConditionVariable())); >- // child_begin()/end() iterates over cond, then, and else stmts. >+DEF_TRAVERSE_STMT(DeclStmt, { >+ for (DeclStmt::decl_iterator I = S->decl_begin(), E = S->decl_end(); >+ I != E; ++I) { >+ TRY_TO(TraverseDecl(*I)); >+ } >+ // Suppress the default iteration over child_begin/end by >+ // returning. Here's why: A DeclStmt looks like 'type var [= >+ // initializer]'. The decls above already traverse over the >+ // initializers, so we don't have to do it again (which >+ // child_begin/end would do). >+ return true; > }) > >-DEF_TRAVERSE_STMT(WhileStmt, { >- TRY_TO(TraverseDecl(S->getConditionVariable())); >- // child_begin()/end() iterates over cond, then, and else stmts. >- }) > > // These non-expr stmts (most of them), do not need any action except > // iterating over the children. > DEF_TRAVERSE_STMT(BreakStmt, { }) >+DEF_TRAVERSE_STMT(CXXTryStmt, { }) >+DEF_TRAVERSE_STMT(CaseStmt, { }) > DEF_TRAVERSE_STMT(CompoundStmt, { }) > DEF_TRAVERSE_STMT(ContinueStmt, { }) >-DEF_TRAVERSE_STMT(CXXTryStmt, { }) >-DEF_TRAVERSE_STMT(DeclStmt, { }) >+DEF_TRAVERSE_STMT(DefaultStmt, { }) > DEF_TRAVERSE_STMT(DoStmt, { }) >+DEF_TRAVERSE_STMT(ForStmt, { }) > DEF_TRAVERSE_STMT(GotoStmt, { }) >+DEF_TRAVERSE_STMT(IfStmt, { }) > DEF_TRAVERSE_STMT(IndirectGotoStmt, { }) > DEF_TRAVERSE_STMT(LabelStmt, { }) > DEF_TRAVERSE_STMT(NullStmt, { }) >@@ -1515,10 +1515,10 @@ > DEF_TRAVERSE_STMT(ObjCAtTryStmt, { }) > DEF_TRAVERSE_STMT(ObjCForCollectionStmt, { }) > DEF_TRAVERSE_STMT(ReturnStmt, { }) >-DEF_TRAVERSE_STMT(SwitchStmt, { }) > DEF_TRAVERSE_STMT(SwitchCase, { }) >-DEF_TRAVERSE_STMT(CaseStmt, { }) >-DEF_TRAVERSE_STMT(DefaultStmt, { }) >+DEF_TRAVERSE_STMT(SwitchStmt, { }) >+DEF_TRAVERSE_STMT(WhileStmt, { }) >+ > > DEF_TRAVERSE_STMT(CXXDependentScopeMemberExpr, { > if (S->hasExplicitTemplateArgs()) { > > >_______________________________________________ >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
