Re: proposed gnulib-related additions to Autoconf

[Ralf Wildenhues]

Hi Paul,

* Paul Eggert wrote on Wed, Mar 01, 2006 at 01:48:54AM CET:
>
>       Import macros from gnulib (often changing their name).
*snip*

And another round of review for your large patch:

> +AU_DEFUN([AC_C_LONG_DOUBLE],
> +  [
> +    AC_TYPE_LONG_DOUBLE_WIDER
> +    if test $ac_cv_type_long_double_wider = yes; then
> +      AC_DEFINE([HAVE_LONG_DOUBLE], 1,
> +     [Define to 1 if the type `long double' works and has more range or
> +      precision than `double'.])
> +    fi
> +  ],
> +  [The macro `AC_C_LONG_DOUBLE' is obsolete.
> +You should use `AC_TYPE_LONG_DOUBLE' or `AC_TYPE_LONG_DOUBLE_WIDER' instead.]
> +)

If it's not too much work for you, it would be great if you could change
the end of the macro to
  )# AC_C_LONG_DOUBLE

and similarly for the other macros, at least the longer ones.  If you
rather prefer, I can post a patch to this end after yours is applied.
It helps navigation.


> +# AC_C_TYPE_RANGE_INTEGER(TYPE, [MIN-VARIABLE], MAX-VARIABLE,
> +#                      [INCLUDES = DEFAULT-INCLUDES])
> +# -----------------------------------------------------------
> +# Compute the bounds of the integer TYPE and define MIN-VARIABLE and
> +# MAX-VARIABLE to those bounds.  The bounds are expressions that are
> +# suitable for use in the preprocessor.  If MIN-VARIABLE is absent, do
> +# not define it.  If either variable is already defined by INCLUDES, do
> +# not define it.  If TYPE does not work, do not define either variable.
> +# Works OK if cross compiling.
> +m4_define([AC_C_TYPE_RANGE_INTEGER],

This needs to be AC_DEFUNed.

> +[
> +  AC_CACHE_CHECK([for $3], [ac_cv_value_$3],
> +    [ac_cv_value_$3=no
> +     ac_signbits=
> +     AC_COMPILE_IFELSE(
> +       [AC_LANG_BOOL_COMPILE_TRY([AC_INCLUDES_DEFAULT([$4])], [($1) -1 < 
> 0])],
> +       [ac_signbits=1; ac_suffix=; ac_unsigned=],
> +       [AC_COMPILE_IFELSE(
> +       [AC_LANG_BOOL_COMPILE_TRY(
> +          [AC_INCLUDES_DEFAULT([$4])], [0 < ($1) -1])],
> +       [ac_signbits=0; ac_suffix=U; ac_unsigned=unsigned])])
> +     if test -n "$ac_signbits"; then
> +       AC_COMPILE_IFELSE(
> +      [AC_LANG_BOOL_COMPILE_TRY([AC_INCLUDES_DEFAULT([$4])], [$3 == $3])],
> +      [ac_cv_value_$3=yes],
> +      [for ac_type in \

ac_1 should be initialized to 1 right before this loop; even if the
other change below is made (otherwise for `short' it could end up empty).

> +           "$ac_unsigned int" \
> +           "$ac_unsigned long int" \
> +           "$ac_unsigned long long int"; do

Couldn't the type we are looking at be incompatible with any of these,
say an intmax_t?  at_1="${ac_suffix}INTMAX_C(1)" would come in handy
below then, if it were defined.

> +         AC_COMPILE_IFELSE(
> +           [AC_LANG_PROGRAM(
> +              [$4
> +               extern $ac_type foo;

That thing should not be called foo.  Lucky as I was, I tested with 
  AC_C_TYPE_RANGE_INTEGER([foo], [FOO_MIN], [FOO_MAX],
                          [typedef unsigned long long foo;])

without really thinking much.  :-)
I'd suggest ac__var_ instead, something a user is quite unlikely to
typedef herself.

> +               extern $1 foo;],
> +              [return !foo;])],
> +           [ac_1=$ac_1$ac_suffix

I think this is better written as
               ac_1=1$ac_suffix

but with the other change above it's ok (albeit needlessly unobvious
IMHO).

> +            case $ac_type in #(
> +            *'long long'*) ac_1=${ac_1}LL;; #(
> +            *long*) ac_1=${ac_1}L;;
> +            esac
> +            break])
> +       done
> +
> +       ac_bits=8

I'm a bit on shaky grounds here, but: if we know we need long or long
long above, can't we increase the minimum ac_bits we start off with, to
32 and 64, respectively, to skip a couple of compilations?
(If that were the case, ac_bits could be set parallel to ac_1).
Bit fields are not getting near this macro anyway.

> +       ac_shiftbits=`expr $ac_bits - 1 - $ac_signbits`
> +       ac_max="(((($ac_1 << $ac_shiftbits) - 1) << 1) + 1)"


The next two block could use just a binary search after the initial
doubling and do things once.  Pure optimization though.

| +       # Double the number of bits until this fails.
| +       while :; do
| +         ac_bits1=`expr $ac_bits '*' 2`
| +         ac_shiftbits=`expr $ac_bits1 - 1 - $ac_signbits`
| +         ac_max1="(((($ac_1 << $ac_shiftbits) - 1) << 1) + 1)"
| +
| +         AC_COMPILE_IFELSE(
| +           [AC_LANG_BOOL_COMPILE_TRY([AC_INCLUDES_DEFAULT([$4])],
| +              [$ac_max < $ac_max1 && ($1) $ac_max1 == $ac_max1])],
| +           [ac_bits=$ac_bits1; ac_max=$ac_max1],
| +           [break])
| +       done
| +
| +       # Add 1 to the number of bits until this fails.
| +       while :; do
| +         ac_bits1=`expr $ac_bits + 1`

Erm, ac_bits won't ever be changed in this loop, so this will loop
forever, iff the loop would have to be executed more than once.

| +         ac_shiftbits=`expr $ac_bits1 - 1 - $ac_signbits`
| +         ac_max1="(((($ac_1 << $ac_shiftbits) - 1) << 1) + 1)"
| +
| +         AC_COMPILE_IFELSE(
| +           [AC_LANG_BOOL_COMPILE_TRY([AC_INCLUDES_DEFAULT([$4])],
| +              [$ac_max < $ac_max1 && ($1) $ac_max1 == $ac_max1])],
| +           [ac_max=$ac_max1],
| +           [break])
| +       done

> +
> +       ac_cv_value_$3=$ac_max])
> +     fi])
*snip*

If you don't mind, I'll suggest a rewrite and some factorization of this
macro that also allows to get at the number of value bits (may be a day
or two though).

> +# AC_TYPE_RANGE_LONG_LONG_INT
> +# ---------------------------
> +AC_DEFUN([AC_TYPE_RANGE_LONG_LONG_INT],
> +  [AC_C_TYPE_RANGE_INTEGER([long long int], [LLONG_MIN], [LLONG_MAX])])
> +
> +
> +# AC_TYPE_RANGE_UNSIGNED_LONG_LONG_INT
> +# ------------------------------------
> +AC_DEFUN([AC_TYPE_RANGE_UNSIGNED_LONG_LONG_INT],
> +  [AC_C_TYPE_RANGE_INTEGER([unsigned long long int], [], [ULLONG_MAX])])

Shouldn't these macros include limits.h if present?  Note limits.h is
not in the default includes set, unlike stdint.h, so those macros will
compute LLONG_MIN etc. themselves and the user will later get
redefinitions if she includes limits.h.

Cheers,
Ralf