Re: rewrote DECIMAL_DIGIT_ACCUMULATE to no longer need typeof

[Jim Meyering]

Paul Eggert <address@hidden> wrote:
> While looking into the problems with typeof on Sun's C compiler, I
> noticed a bug in DECIMAL_DIGIT_ACCUMULATE: it assumed that the type of
> the accumulator never promoted to int.  While fixing this, I noticed
> that I could rewrite DECIMAL_DIGIT_ACCUMULATE so that it didn't need
> typeof at all.  This simplifies configuration, and it improves checking
> on compilers that don't have typeof.  I installed this:
>
> 2005-07-04  Paul Eggert  <address@hidden>
>
>       * m4/prereq.m4 (gl_PREREQ): Don't require gl_TYPEOF; no longer needed.
>       * m4/typeof.m4: Remove; no longer needed.
>       * src/system.h (VERIFY_W_TYPEOF): Remove; no longer needed.
>       (DECIMAL_DIGIT_ACCUMULATE): Change last arg from T's maximum value
>       to T itself.  All callers changed.  Check that T is unsigned, and
>       that Accum is of type T.  This fixes a bug in the unlikely case
>       where SIZE_MAX <= INT_MAX, and it no longer requires typeof to do
>       the proper validity checks.

I do like the idea, and would indeed like to avoid using typeof,
but your new test for matching types doesn't work.
Now, a type mismatch merely evokes a warning from gcc.
It should evoke a failure.

  $ cat k.c
  int
  foo (void)
  {
    int p;
    return &p == (long *) 0;
  }
  $ gcc -c k.c
  k.c: In function 'foo':
  k.c:5: warning: comparison of distinct pointer types lacks a cast

Admittedly, that'd work for me, since I always run `make distcheck'
which does compile with gcc -Werror, but I'd prefer a test that
evokes a failure even without -Werror, even if it means relying on
__typeof__.

Also, this new version of the macro always fails for signed accumulators.
Why?  That restriction seems unnecessary: just use
`TYPE_MAXIMUM (Type)' in place of `(Type) -1'.

...
> Index: src/system.h
> ===================================================================
...
> +/* If 10*Accum + Digit_val is larger than the maximum value for Type,
> +   then don't update Accum and return false to indicate it would
> +   overflow.  Otherwise, set Accum to that new value and return true.
> +   Verify at compile-time that Type is Accum's type, and that Type is
> +   unsigned.  Accum must be an object, so that we can take its address.
> +   Accum and Digit_val may be evaluated multiple times.  */
>  
> -/* If 10*Accum+Digit_val is larger than Type_max, then don't update Accum
> -   and return zero to indicate it would overflow.  Otherwise, set Accum to
> -   that new value and return nonzero.  With a compiler that provides the
> -   __typeof__ operator, perform a compile-time check to verify that the
> -   specified Type_max value is the same as the constant derived from the
> -   type of Accum.  */
> -#define DECIMAL_DIGIT_ACCUMULATE(Accum, Digit_val, Type_max)         \
> +#define DECIMAL_DIGIT_ACCUMULATE(Accum, Digit_val, Type)             \
>    (                                                                  \
> -   /* Ensure that Type_max is the maximum value of Accum.  */                
> \
> -   VERIFY_W_TYPEOF (TYPE_MAXIMUM (__typeof__ (Accum)) == (Type_max)),        
> \
> -   ((Type_max) / 10 < Accum || Accum * 10 + (Digit_val) < Accum              
> \
> -            ? 0 : ((Accum = Accum * 10 + (Digit_val)), 1))           \
> +   (void) (&(Accum) == (Type *) NULL),  /* The type matches.  */     \
> +   verify_expr (! TYPE_SIGNED (Type)),  /* The type is unsigned.  */ \
> +   (((Type) -1 / 10 < (Accum)                                                
> \
> +     || (Type) ((Accum) * 10 + (Digit_val)) < (Accum))                       
> \
> +    ? false : (((Accum) = (Accum) * 10 + (Digit_val)), true))                
> \
>    )