[cfe-commits] r109590 - /cfe/trunk/include/clang/AST/RecursiveASTVisitor.h

Nick Lewycky nlewycky at google.com
Fri Jul 30 16:05:04 PDT 2010


On 28 July 2010 08:54, Craig Silverstein <csilvers2000 at yahoo.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100730/77ecd7e1/attachment.html>


More information about the cfe-commits mailing list