Re: [PATCH] More value_range API cleanup

[Aldy Hernandez <aldyh at redhat dot com>]

> > The tricky part starts in the prologue for
> >
> >    if (vr0->undefined_p ())
> >      {
> >        vr0->deep_copy (vr1);
> >        return;
> >      }
> >
> > but yes, we probably can factor out a bit more common code
> > here.  I'll see to followup with more minor cleanups this
> > week (noticed a few details myself).
>
> Like this?  (untested)

I would inline value_range_base::union_helper into 
value_range_base::union_, and remove all the undefined/varying/etc stuff 
from value_range::union_.

If should work because upon return from value_range_base::union_, in the 
this->undefined_p case, the base class will copy everything but the 
equivalences.  Then the derived union_ only has to nuke the equivalences 
if this->undefined or this->varying, and the equivalences' IOR just works.

For instance, in the case where other->undefined_p, there shouldn't be 
anything in the equivalences so the IOR won't copy anything to this as 
expected.  Similarly for this->varying_p.

In the case of other->varying, this will already been set to varying so 
neither this nor other should have anything in their equivalence fields, 
so the IOR won't do anything.

I think I covered all of them...the bitmap math should just work.  What 
do you think?

Finally, as I've hinted before, I think we need to be careful that any 
time we change state to VARYING / UNDEFINED from a base method, that the 
derived class is in a sane state (there are no equivalences set per the 
API contract).  This was not originally enforced in VRP, and I wouldn't 
be surprised if there are dragons if we enforce honesty.  I suppose, 
since we have an API, we could enforce this lazily: any time equiv() is 
called, clear the equivalences or return NULL if it's varying or 
undefined?  Just a thought.

Thanks for more cleanups in this area.
Aldy