[LLVMdev] Some df_iterator and po_iterator issues

[Olaf Krzikalla]

Hi @llvm,

below is a copy of a message I sent to clang a hour before. I guess it's 
more appropriate here.

--snip--

While trying to eleminate as much std::tr1::function as possible I 
stumbled over a design flaw in llvm::df_iterator.
Consider the following code:

void for_all_stmts(Stmt* S, const std::tr1::function<void(Stmt*)>& fn)
{
  if (S)
  {
    fn(S);
    for (Stmt::child_iterator i = S->child_begin(), e = S->child_end(); 
i != e; ++i)
    {
      for_all_stmts(*i, fn);
    }
  }
}

In a first step I want to replace this with:

void for_all_stmts(Stmt* S, const std::tr1::function<void(Stmt*)>& fn)
{
  for (llvm::df_iterator<Stmt*> i = llvm::df_begin(S), e = 
llvm::df_end(S); i != e; ++i)
  {
    if (*i)  fn(*i);
  }
}

However if fn replaces childrens of a just processed statement (which 
happens a lot), the iteration may crash. Looking at df_iterator reveals 
the reason: the first child of a particular statement is stored too 
early for this kind of usage. IMHO this could be fixed by delaying the 
computation of the first child until it's needed (that is in the 
preincrement operator). The only open question would be: how do we mark 
the children iterator invalid before its first use.

--snip--

I added two patches to this mail. The first one (dfi) is a proposal 
fixing the problem described in the cut-out above.
The second one (poi) have to be applied anyway, as it fixes a compile 
bug rendering po_ext_iterator currently unusable.
However this second issued occured as part of another problem: a child 
Stmt of a Stmt in a clang AST can be 0 (e.g. in for(;;)).
I could work around it by providing po_ext_iterator an external set of 
visited nodes and just inserted 0 beforehand. However I'm quite sure it 
would be a better idea to catch the 0 case in po_iterator and 
df_iterator at all. Otherwise using these classes become tedious very 
quickly.

Best
Olaf Krzikalla

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: dfi.patch
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090615/36dd77a4/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: poi.patch
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090615/36dd77a4/attachment-0001.ksh>