[cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

[Lawrence Crowl]

On 8/7/12, Richard Guenther <richard.guenther@gmail.com> wrote:
> On Tue, Aug 7, 2012 at 2:35 AM, Lawrence Crowl <crowl@google.com> wrote:
> > Convert double_int from a struct with function into a class with
> > operators and methods.
> >
> > This patch adds the methods and operators.  In general functions of
> > the form "double_int_whatever" become member functions "whatever" or,
> > when possible, operators.
> >
> > Every attempt has been made to preserve the existing algorithms, even
> > at the expense of some optimization opportunities.  Some examples:
> >
> >   The ext operation takes a value and returns a value.  However, that
> >   return value is usually assigned to the original variable.  An
> >   operation that modified a variable would be more efficient.
>
> That's not always the case though and I think the interface should be
> consistent with existing behavior to avoid errors creeping in during the
> transition.

We should probably think about naming conventions for mutating operations,
as I expect we will want them eventually.

> >   In some cases, an outer sign-specific function calls an inner
> >   function with the sign as a parameter, which then decides which
> >   implementation to do.  Decisions should not be artificially
> >   introduced, and the implementation of each case should be exposed as
> >   a separate routine.
> >
> >   The existing operations are implemented in terms of the new
> >   operations, which necessarily adds a layer between the new code and
> >   the existing users.  Once all files have migrated, this layer will
> >   be removed.
> >
> >   There are several existing operations implemented in terms of even
> >   older legacy operations.  This extra layer has not been removed.
> >
> > On occasion though, parameterized functions are often called
> > with a constant argments.  To support static statement of intent,
> > and potentially faster code in the future, there are several new
> > unparameterized member functions.  Some examples:
> >
> >   Four routines now encode both 'arithmetic or logical' and 'right or
> >   left' shift as part of the funciton name.
> >
> >   Four routines now encode both 'signed or unsigned' and 'less than or
> >   greater than' as part of the function name.
>
> For most parts overloads that take an (unsigned) HOST_WIDE_INT argument
> would be nice, as well as the ability to say dbl + 1.

Hm.  There seems to be significant opinion that there should not be any
implicit conversions.  I am okay with operations as above, but would like
to hear the opinions of others.

> > -typedef struct
> > +typedef struct double_int
> >  {
> > +public:
> > +  /* Normally, we would define constructors to create instances.
> > +     Two things prevent us from doing so.
> > +     First, defining a constructor makes the class non-POD in C++03,
> > +     and we certainly want double_int to be a POD.
> > +     Second, the GCC conding conventions prefer explicit conversion,
> > +     and explicit conversion operators are not available until C++11.
> > */
> > +
> > +  static double_int make (unsigned HOST_WIDE_INT cst);
> > +  static double_int make (HOST_WIDE_INT cst);
> > +  static double_int make (unsigned int cst);
> > +  static double_int make (int cst);
>
> Did we somehow decide to not allow constructors?  It's odd to convert to
> C++ and end up with static member functions resembling them ...

Constructors are allowed, but PODs are often passed more efficiently.
That property seemed particularly important for double_int.

> Also I believe the conversion above introduces possible migration errors.
> Think of a previous
>
>  HOST_WIDE_INT a;
>  double_int d = uhwi_to_double_int (a);
>
> if you write that now as
>
>  HOST_WIDE_INT a;
>  double_int d = double_int::make (a);
>
> you get the effect of shwi_to_double_int.  Oops.

Hm.  I think the code was more likely to be wrong originally,
but I take your point.

> So as an intermediate
> I'd like you _not_ to introduce the make () overloads.

Okay.

> Btw, if HOST_WIDE_INT == int the above won't even compile.

Is that true of any host?  I am not aware of any.  Anyway, it is
moot if we remove the overloads.

-- 
Lawrence Crowl