Re: [PATCH 1/2] Update iterator of next

[Jan Hubicka <hubicka at ucw dot cz>]

> Hi,
> 
> On Fri, Nov 15 2019, luoxhu wrote:
> >
> > Sorry to break the bootstrap.  This was my lack of consideration when
> > splitting this small piece of code from the previous patch, the line
> > should be in the first line of the second loop.   Could you please add
> > some comments that edge may be freed in resolve_speculation()?
> > Thanks.
> 
> First and foremost, resolve_speculation should not free its this
> pointer.  This is the third time it has caused us a headache in the last
> few months.  If we cannot think of a better solution in the next week or
> three, I will revert it back to a non-member function in this stage 3

The bug in the walker loop here IMO predated C++ conversion, but to
avoid member function freeing THIS we probably want to have:

cgraph_edge::remove (e),
symtab_node::remove (e),
ipa_ref::remove (ref),
cgraph_node::resolve_speculation (e, target)

I suppose good time for the conversion would be after remaining patches
are merget. If you would have time to do that even better ;)

> ...and then we can (and should and will) document that it can free the
> edge.
It speaks about removing edges and return value, 

     Speculative call edge turned out to be direct call to CALLEE_DECL.         
     Remove the speculative call sequence and return edge representing the call.
     It is up to caller to redirect the call as appropriate.

We could add
     Edge E may be freed if it is resolved to other edge of the
     speculative call.

Honza
> 
> Martin