[LLVMdev] subtle problem with inst_iterator

[Vladimir Prus]

Chris Lattner wrote:
> On Fri, 23 Apr 2004, Vladimir Prus wrote:
> > and since result of *it is considered to be rvalue it can't be accepted
> > by this operator. The complete discussion is in
> >
> >     http://std.dkuug.dk/jtc1/sc22/wg21/docs/papers/2002/n1385.htm
> >
> > I'd suggest to apply the following patch which makes operator* return
> > reference to pointer. It makes my code compile and the rest of LLVM
> > compiles too. Comments?
>
> Hrm, I'm not sure about this at all.  In particular ilists have some funny
> semantics that you should know about.  In particular, the instruction list
> is more like a std::list<Instruction> (but that allows polymorphism) than
> a std::list<Instruction*>, 

Yea, I've noticed that. However, it looks like inst_iterator is iterator over 
pointers. Oh, wait a minite, that's the current code:

  inline IIty operator*()  const { return BI; }
  inline IIty operator->() const { return operator*(); }

So operator* works as if value_type is Instruction*, but operator-> works as 
if value_type is Instruction. Hmm ;-)

> so it's not possible to just assign a new
> instruction* through an iterator.  In other words:  *iit = new
> Instruction(...) doesn't make any sense.
>
> Probably the right thing to do would be to make the inst_iterator return a
> reference to the *instruction* itself, rather than a reference to the
> pointer.  This would make it work the same as standard ilist iterators.

Yes, I though about this appoach as well but decided it can be too disturbing 
for other code.

> The reason why we have it return pointers right now is to populate
> worklists, which allows us to do:
>
> std::set<Instruction*> WorkList(inst_begin(F), inst_end(F));

Maybe this can be rewritten like;

    std::set<Instruction*> WorkList;
    std::transform(insn_begin(F), insn_end(F), 
	inserter(WorkList, WorkList.begin(), take_address);

given proper definition of take_addres. Or maybe manual initialization will 
okay. 

So, if you think making inst_iterator's operator* return reference to 
instruction is better than making it return reference to pointer, I can take 
a look at this.

- Volodya