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
