Re: SSA range class and removal of VR_ANTI_RANGEs

[Martin Sebor <msebor at gmail dot com>]

On 06/20/2017 02:41 AM, Aldy Hernandez wrote:
> On 05/23/2017 03:26 PM, Martin Sebor wrote:
> > On 05/23/2017 04:48 AM, Aldy Hernandez wrote:
>
> > +  void Union (wide_int x, wide_int y);
> > +  bool Union (const irange &r);
> > +  bool Union (const irange &r1, const irange &r2);
> > +
> > +  // THIS = THIS ^ [X,Y].  Return TRUE if result is non-empty.
> > +  bool Intersect (wide_int x, wide_int y, bool readonly = false);
> > +  // THIS = THIS ^ R.  Return TRUE if result is non-empty.
> > +  // THIS = R1 ^ R2.  Return TRUE if result is non-empty.
> > +  bool Intersect (const irange &r1, const irange &r2, bool readonly =
> > false);
> > +  // Return TRUE if THIS ^ R will be non-empty.
> > +  bool Intersect_p (const irange &r)
> > +    { return Intersect (r, /*readonly=*/true); }
> >
> > I would suggest the following changes to Union, Intersect, and Not:
> >
> > 1) Define all three members without the readonly argument and
> >     returning irange& (i.e., *this).  The return value can be
> >     used wherever irange& is expected, and the is_empty() member
> >     function can be called on it to obtain the same result.  E.g.,
> >     Intersect A with B, storing the result in A:
> >
> >     irange A, B;
> >     if (A.Intersect (B).is_empty ()) { ... }
> >
> > 2) Add non-members like so:
> >
> >    irange range_union (const irange &lhs, const irange &rhs)
> >    {
> >      return irange (lhs).Union (rhs);
> >    }
> >
> >    and find out if the union of A or B is empty without modifying
> >    either argument:
> >
> >    irange A, B;
> >    if (range_union (A, B).is_empty ()) { ... }
>
> Perhaps we could provide an implicit conversion from irange to bool such
> that we could write:
>
> if (range_union (A, B)) { ... }
>
> as well as being able to write:
>
> if (!range_union (A, B).is_empty ()) { ... }
>
> That is, have range_union() return an irange as suggested, but have a
> bool overload (or whatever the C++ nomenclature is) such that converting
> an irange to a bool is interpreted as ``nitems != 0''.
>
> Is this acceptable C++ practice?

Implicit conversion to bool is a common way of testing validity
but I don't think it would be too surprising to use it as a test
for non-emptiness.  An alternative to consider is to provide
an implicit conversion to an unsigned integer (instead of
num_ranges()(*)) and have it return the number of ranges.  That
will make it possible to do the same thing as above while also
simplifying the API.

Martin

[*] FWIW, there's nothing wrong with the name num_ranges() but
those familiar with the C++ standard library are going to be
accustomed to size() as the name of a function that returns
the number of elements in a container.  Since the irange class
is an ordered sequence of ranges, size() would work for it too.

PS Thinking of the irange class as a container of ranges suggests
the design might benefit from introducing a simple lower-level
abstraction (class) for a single contiguous range.