[LLVMdev] Patch for llvm::DepthFirstIterator.h and llvm::PostOrderIterator.h

[Dan Gohman]

Hi Olaf,

This patch looks good to me.  I just have a few minor comments:

 > +  inline df_iterator() { CurrentTopNode = 0; /* End is when stack  
is empty */ }

Should the comment here be updated to say that the End
is reached when the stack is empty and when CurrentTopNode
is null?

 > +  inline void toNext()
 > +  {

LLVM style puts the open brace on the same line as the function name.

 > +  inline _Self& skipChilds() {  // skips all childs of the current  
node and traverses to next node

The plural of "child" is "children"; please rename this function
accordingly.  Also, please format this line and a few others so
that they don't exceed 80 columns.

Thanks,

Dan

On Jun 26, 2009, at 5:32 AM, Olaf Krzikalla wrote:

> Hi @clang and @llvm,
>
> attached you'll find a patch dealing with some iterator issues I  
> already mentioned in both lists. Since there was no reaction I cross- 
> post again - now IMHO production-ready code.
> The patch is considered to get checked-in out of the box. It should  
> not affect the behavior of existing and working code. I really need  
> it for clang AST processing.
>
> Changes:
> 1. Both iterators now can deal with sub-nodes==0 - they just skip  
> them silently. For AST processing this is badly needed as you  
> sometimes have 0-nodes in the AST (e.g. in 'for(;;) {}'). Until now  
> both iterators crash if they traverse a sub-node==0.
> 2. In llvm::PostOrderIterator.h I fixed a compile bug which occured  
> if used with external storage (which apparently nobody does until  
> now).
> 3. I changed the behavior of llvm::DepthFirstIterator.h regarding  
> the retrieving of the first child. This is now retrieved in exactly  
> that moment it's needed. Until now it was retrieved too early, thus  
> you actually couldn't change the childs of a just processed node. I  
> can't think of an insane working example, which would break due to  
> this change.
> 4. I added a public skipChilds member function to df_iterator which  
> acts similiar to operator++. However it skips all childs of the  
> currently processed node and returns *this. You can use it like in
> for (i = df_begin(), e = df_end(); i!=e;)
> {
> foo() ? i.skipChilds() : ++i;
> }
>
>
> Best
> Olaf Krzikalla
> <llvm-iterator.patch>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev