Re: UNSIGNED_ITYPE_MAX signed?

[Dirk Herrmann]

Hello folks,

after having moved to the south of Germany, I hope to find some time for
guile again...  There's a lot of things to do on my guile list :-)

On 16 Dec 2001, Marius Vollmer wrote:

> > Without my patch, I still was getting warning messages when compiling with
> > -Wall and -W, although I don't remember what kind of warning it was.  My
> > version of gcc is "2.95.3".
> 
> Sounds plausible, from looking at the code.  Are you still having
> problems with this?

A) When compiling numbers.c with the CVS version of num2integral.i.c I
get the following warnings:

gcc -DHAVE_CONFIG_H -I.. -I./.. -g -O2 -Wall -W -DSCM_DEBUG=1 
-Wp,-MD,.deps/numbers.pp -c numbers.c  -fPIC -DPIC -o numbers.o
In file included from numbers.c:4355:
../libguile/num2integral.i.c: In function `scm_num2uint':
../libguile/num2integral.i.c:36: warning: comparison between signed and unsigned
In file included from numbers.c:4370:
../libguile/num2integral.i.c: In function `scm_num2ulong':
../libguile/num2integral.i.c:36: warning: comparison between signed and unsigned
In file included from numbers.c:4386:
../libguile/num2integral.i.c: In function `scm_num2size':
../libguile/num2integral.i.c:36: warning: comparison between signed and unsigned
In file included from numbers.c:4407:
../libguile/num2integral.i.c: In function `scm_num2ulong_long':
../libguile/num2integral.i.c:36: warning: comparison between signed and unsigned
mv -f numbers.o .libs/numbers.lo

B) after applying the following patch ...

> cvs diff -r HEAD num2integral.i.c 
Index: num2integral.i.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/num2integral.i.c,v
retrieving revision 1.13
diff -u -r1.13 num2integral.i.c
--- num2integral.i.c    12 Nov 2001 01:12:37 -0000      1.13
+++ num2integral.i.c    23 Feb 2002 00:32:34 -0000
@@ -27,17 +27,21 @@
         scm_out_of_range (s_caller, num);
 #endif
     
-      if (sizeof (ITYPE) >= sizeof (scm_t_signed_bits))
-        /* can't fit anything too big for this type in an inum
-           anyway */
-        return (ITYPE) n;
-      else
-        { /* an inum can be out of range, so check */
-         if (((ITYPE)n) != n)
-            scm_out_of_range (s_caller, num);
-          else
-            return (ITYPE) n;
-        }
+#if SIZEOF_ITYPE >= SIZEOF_SCM_T_BITS
+      /* the target type is large enough to hold any possible inum */
+      return (ITYPE) n;
+#else
+      /* an inum can be out of range, so check */
+#ifdef UNSIGNED
+      /* n is known to be >= 0 */
+      if ((scm_t_bits) n > UNSIGNED_ITYPE_MAX)
+       scm_out_of_range (s_caller, num);
+#else
+      if (((ITYPE)n) != n)
+       scm_out_of_range (s_caller, num);
+#endif
+      return (ITYPE) n;
+#endif /* SIZEOF_ITYPE >= SIZEOF_SCM_T_BITS */
     }
   else if (SCM_BIGP (num))
     { /* bignum */

... I still get one warning with gcc 2.95.3, which is surprising:

gcc -DHAVE_CONFIG_H -I.. -I./.. -g -O2 -Wall -W -DSCM_DEBUG=1
-Wp,-MD,.deps/numbers.pp -c numbers.c  -fPIC -DPIC -o numbers.o
In file included from numbers.c:4340:
../libguile/num2integral.i.c: In function `scm_num2ushort':
../libguile/num2integral.i.c:37: warning: comparison between signed and unsigned

C) When changing the patch under B) slightly, coercing UNSIGNED_ITYPE_MAX  
to an unsigned value (which it should, IMO, already have been) ...

> cvs diff -r HEAD num2integral.i.c 
Index: num2integral.i.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/num2integral.i.c,v
retrieving revision 1.13
diff -u -r1.13 num2integral.i.c
--- num2integral.i.c    12 Nov 2001 01:12:37 -0000      1.13
+++ num2integral.i.c    23 Feb 2002 00:37:49 -0000
@@ -27,17 +27,22 @@
         scm_out_of_range (s_caller, num);
 #endif
     
-      if (sizeof (ITYPE) >= sizeof (scm_t_signed_bits))
-        /* can't fit anything too big for this type in an inum
-           anyway */
-        return (ITYPE) n;
-      else
-        { /* an inum can be out of range, so check */
-         if (((ITYPE)n) != n)
-            scm_out_of_range (s_caller, num);
-          else
-            return (ITYPE) n;
-        }
+#if SIZEOF_ITYPE >= SIZEOF_SCM_T_BITS
+      /* the target type is large enough to hold any possible inum */
+      return (ITYPE) n;
+#else
+      /* an inum can be out of range, so check */
+#ifdef UNSIGNED
+      /* n is known to be >= 0 */
+      /* Dirk:FIXME:: UNSIGNED_ITYPE_MAX is a signed value? gcc bug? */
+      if ((scm_t_bits) n > (UNSIGNED_ITYPE) UNSIGNED_ITYPE_MAX)
+       scm_out_of_range (s_caller, num);
+#else
+      if (((ITYPE)n) != n)
+       scm_out_of_range (s_caller, num);
+#endif
+      return (ITYPE) n;
+#endif /* SIZEOF_ITYPE >= SIZEOF_SCM_T_BITS */
     }
   else if (SCM_BIGP (num))
     { /* bignum */

... everything compiles fine without warnings.

Now I would like to apply either patch B) or patch C).  Which one depends
on why the patch B) produces a warning.  Can anyone comment on it?  Is it
a gcc bug (apply patch B, just live with the warning and hope for gcc to
be fixed in a newer version), is the behaviour undefined by ISO C (apply
patch C without FIXME comment), or is gcc correct (apply patch C without
FIXME comment)?

Best regards,
Dirk Herrmann