bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-gnulib] proposed simplification rewrite of stdint module


From: Paul Eggert
Subject: Re: [bug-gnulib] proposed simplification rewrite of stdint module
Date: Thu, 29 Jun 2006 10:32:04 -0700
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)

Bruno Haible <bruno@clisp.org> writes:

>> #if @HAVE_STDINT_H@
>> # if defined __sgi && ! defined __c99 && __STDC_VERSION__ < 199901L
>
> Where does the __STDC_VERSION__ test come from?

I wanted to handle the future case when SGI fixes the bug in their
system.  But you're right, this won't work with GCC today, so I'll
remove that part of the test.  We can worry about SGI future versions
later.

> The last line gives an integer overflow, which some compilers may rightfully
> complain about. I think it will give less warnings when written as
>
>      : ((((zero) + 1) << ((bits) ? (bits) - 1 : 0)) - 1) * 2 + 1)

Hmm, that's not quite right either, surely you meant (bits) - 2.
But I now thought of a simpler definition.  _STDINT_MAX needs only
two arguments, and can be simplified to:

/* _STDINT_MAX relies on the signedness of 'zero' to return the
   correct type.  */
#define _STDINT_MAX(bits, zero) (~ _STDINT_MIN (1, bits, zero))


> on 32-bit platforms, nowadays, 32-bit types are the safest
> if you want speed. On 64-bit platform it's likely the 64-bit types, but
> 32-bit types are usually well supported too.

OK, I'll change it to this, which follows the advice you gave:

   #define int_fast8_t long int
   #define uint_fast8_t unsigned int_fast8_t
   #define int_fast16_t long int
   #define uint_fast16_t unsigned int_fast16_t
   #define int_fast32_t long int
   #define uint_fast32_t unsigned int_fast32_t

> This is lacking the case for _MSC_VER. Why not use a "#ifdef int64_t"
> here?

Thanks.  Mark D. Baushke already raised this issue via private mail
(my answer is in
<http://lists.gnu.org/archive/html/bug-gnulib/2006-06/msg00251.html>)
but your suggested fix is better.

>> #ifdef INT64_C
>> # define INTMAX_C(x)   INT64_C(x)
>> # define UINTMAX_C(x)  UINT64_C(x)
>
> I think it's safer if this would use the same #if condition as the
> definition of uintmax_t.

OK, thanks, I'll do that, as follows:

   #if @HAVE_LONG_LONG_INT@
   # define INTMAX_C(x)   x##LL
   # define UINTMAX_C(x)  x##ULL
   #elif defined int64_t
   # define INTMAX_C(x)   INT64_C(x)
   # define UINTMAX_C(x)  UINT64_C(x)
   #else
   # define INTMAX_C(x)   x##L
   # define UINTMAX_C(x)  x##UL
   #endif

> Why #ifndef here, and #undef for the others? I've seen a BSD system
> a week ago, where wchar_t is a signed 32-bit type and WCHAR_MAX = 0x7fffffff
> and WCHAR_MIN = 0 (!!).

OK, thanks, I didn't know about that.  I'll #undef the MIN and MAX values
for wint_t, wchar_t, ptrdiff_t, size_t, sig_atomic_t.

> IMO this test also needs to
> check that "long long int" is longer than 32 bits or longer than "long int".

OK, thanks, I'll add that by modifying AC_TYPE_LONG_LONG_INT and
AC_TYPE_UNSIGNED_LONG_LONG INT.  This will require a change to
Autoconf.  In the meantime I'll update m4/longlong.m4 and
m4/ulonglong.m4 to override Autoconf 2.60, using the 2.60 API but with
the improved check.  Something like this, for example:

   AC_DEFUN([AC_TYPE_LONG_LONG_INT],
   [
     AC_CACHE_CHECK([for long long int], [ac_cv_type_long_long_int],
       [AC_LINK_IFELSE(
          [AC_LANG_PROGRAM(
             [long long int ll = 9223372036854775807ll;
              long long int nll = -9223372036854775807LL;
              typedef int a[((-9223372036854775807LL < 0
                              && 0 < 9223372036854775807ll)
                             ? 1 : -1)];
              int i = 63;],
             [long long int llmax = 9223372036854775807ll;
              return (ll << 63 | ll >> 63 | ll < i | ll > i
                      | llmax / ll | llmax % ll);])],
          [ac_cv_type_long_long_int=yes],
          [ac_cv_type_long_long_int=no])])
     if test $ac_cv_type_long_long_int = yes; then
       AC_DEFINE([HAVE_LONG_LONG_INT], 1,
         [Define to 1 if the system has the type `long long int'.])
     fi
   ])

This will propagate into some other macros due to the name change, but
that's merely mechanical.


>>   if test $ac_cv_header_inttypes_h = yes; then
>>   if test $ac_cv_header_sys_types_h = yes; then
>>   if test $ac_cv_header_stdint_h = yes; then
>
> Can you please add comments explaining where these variable values come from?

Will do.

>>        if eval test \"\$gl_cv_type_${gltype}_signed\" = yes; then
>
> I got some errors when trying to use 'eval' in this way.

Weird.  I'll rewrite it as you suggest.  If you run into it again,
can you please send a note to bug-autoconf@gnu.org?  I'd like to
write it up under Shellology.

> For completeness, this should also set
>
>     gl_cv_type_size_t_signed=no

Will do.

> Finally, two testsuite updates: Where it uses
>     _STDINT_H_HAVE_INT64
> it now should say
>     _STDINT_H_HAVE_INT64 || defined int64_t
> and similarly
>     _STDINT_H_HAVE_UINT64
> should become
>     _STDINT_H_HAVE_UINT64 || defined uint64_t

Hmm, _STDINT_H_HAVE_UINT64 is no longer defined.  I'll try simpling
use the C99 standard symbols, e.g., "#ifdef INT64_MAX", "#ifdef
UINT64_MAX"; that should be more robust to future changes to the
stdint_.h implementation.

> And it now uses HAVE_WCHAR_T and HAVE_WINT_T, therefore modules/stdint-tests
> should use m4/wchar_t.m4 and m4/wint_t.m4 and invoke these macros.

OK, thanks.

> In a few weeks, after gettext-0.15 is released, I'll also submit a rewrite
> of the 'inttypes' module. The substitute <inttypes.h> includes <stdint.h>,
> therefore at that moment we will have to change the
>    # include <inttypes.h>
> back to
>    # include @FULL_PATH_INTTYPES_H@

OK, thanks, might as well do that now then.

By the way, we need to change these names to not use the word "path",
according to the GNU coding standards.  But that's for a later patch;
this one is already too big.

Also, it is probably time to retire some of the existing code, which
is superseded by all this, e.g., size_max, intmax_t.m4, etc.  But that
can wait until stdint stabilizes.

Thanks very much for the review.  I'll test the revised version for a
bit before installing.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]