Relaxing -Wsign-compare

Relaxing -Wsign-compare

[Morten Welinder]

IMHO, the -Wsign-compare is a bit too agressive to be really useful.
Consider this example:

-----------------------------------------------------------------------------
struct {
  int a, b;
} foo [] = {
  {1,2}, {3,4}
};

int
main (int argc, char **argv)
{
  int i;

  for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i++)
    ;

  return 0;
}
-----------------------------------------------------------------------------


> gcc-3.0 -Wsign-compare signed.c
signed.c: In function `main':
signed.c:12: warning: comparison between signed and unsigned

A similar warning can be had with typical MIN and MAX macros when
used with an unsigned expression and an explicit integer, say
MAX (expr, 42).

Would it be reasonable to silence this warning when one of the sides is
a constant (in the C meaning) that fits well within the range of the
other side's type?

Morten

Re: Relaxing -Wsign-compare

[Neil Booth]

Morten Welinder wrote:-

> IMHO, the -Wsign-compare is a bit too agressive to be really useful.
> Consider this example:
> 
> -----------------------------------------------------------------------------
> struct {
>   int a, b;
> } foo [] = {
>   {1,2}, {3,4}
> };
> 
> int
> main (int argc, char **argv)
> {
>   int i;
> 
>   for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i++)
>     ;
> 
>   return 0;
> }
> -----------------------------------------------------------------------------

I don't think you've chosen a good example -- I'd claim that i should
be an unsigned int.  Personally, it bugs me that people default to
loops with int, when 95% of the time they're looping with unsigned
quantities.  It can even give you better code to be looping unsigned,
since some arithmetic operations can be more efficiently encoded if
the compiler knows they are on unsigned quantities.

And signed lengths, for that matter.

> signed.c: In function `main':
> signed.c:12: warning: comparison between signed and unsigned
> 
> A similar warning can be had with typical MIN and MAX macros when
> used with an unsigned expression and an explicit integer, say
> MAX (expr, 42).

I think Kaveh has done some work on the compiler to improve this area
in 3.1.

Neil.

Re: Relaxing -Wsign-compare

[Morten Welinder]

   >   for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i++)

[...]


   I don't think you've chosen a good example -- I'd claim that i should
   be an unsigned int.  Personally, it bugs me that people default to
   loops with int, when 95% of the time they're looping with unsigned
   quantities.

It ought not matter whether the type was "int" or "unsigned int".  In
both cases, the compiler ought to infer that the value will fit in
either type.  Then it can pick whatever instruction sequence it feels
like.  Ah, all the things one could do with infinite time and money...

   It can even give you better code to be looping unsigned,
   since some arithmetic operations can be more efficiently encoded if
   the compiler knows they are on unsigned quantities.

Or the other way around, perhaps.

   I think Kaveh has done some work on the compiler to improve this area
   in 3.1.

Great, thanks!

Morten

Re: Relaxing -Wsign-compare

[Richard Henderson]

On Thu, Sep 20, 2001 at 07:41:45PM +0100, Neil Booth wrote:
> I think Kaveh has done some work on the compiler to improve this area
> in 3.1.

What Kaveh did was the other way around -- suppressing the
warning for 

	unsigned int i;
	if (i < 6) ...

In this example "6" is signed int, and gets promoted to unsigned.
The examples Kaveh was working with were slightly more complicated.  ;-)


r~

Re: Relaxing -Wsign-compare

[Frank Klemm]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4068 bytes --]

On Thu, Sep 20, 2001 at 03:45:27PM -0000, Morten Welinder wrote:
> 
> IMHO, the -Wsign-compare is a bit too agressive to be really useful.
> Consider this example:
> 
> -----------------------------------------------------------------------------
> struct {
>   int a, b;
> } foo [] = {
>   {1,2}, {3,4}
> };
> 
> int
> main (int argc, char **argv)
> {
>   int i;
> 
>   for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i++)
>     ;
> 
>   return 0;
> }
> -----------------------------------------------------------------------------
> 
> > gcc-3.0 -Wsign-compare signed.c
> signed.c: In function `main':
> signed.c:12: warning: comparison between signed and unsigned
> 
> A similar warning can be had with typical MIN and MAX macros when
> used with an unsigned expression and an explicit integer, say
> MAX (expr, 42).
> 
> Would it be reasonable to silence this warning when one of the sides is
> a constant (in the C meaning) that fits well within the range of the
> other side's type?
> 
------------------------------------------------------------------------------
16 bit integer CPU/Compiler:

char  foo [32000];

int
main ( int argc, char** argv )
{
    int  i;
 
    for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i += 1000 )
        ;
    return 0;
}

------------------------------------------------------------------------------
BTW the right coding is:

char  foo [32000];

int
main ( int argc, char** argv )
{
    size_t  i;
 
    for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i += 1000 )
        ;
    return 0;
}

This will also works correctly in 2010 in the following context:

char  foo [4_700_000_000];	// small buffer or memory-mapped DVD

int
main ( int argc, char** argv )
{
    size_t  i;
 
    for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i += 1000 )
        ;
    return 0;
}

--------------------------------------------------------------------------------

The main problem I have with signed vs. unsigned are normal strings.
In Germany we have so called modified vowels, also other languages have
modified letters. These are part of ISO-646 aka ISO-8859-1. But it is
necessary to use 'unsigned char' for characters (otherwise you tap a rich
pool of errors):

   const unsigned char*  text = "Erdbärtörtchen";

When you are using strcmp(), it complains about sign issue:

  const unsigned char*  cmpstr = "Eichhörnchenpfötchen";

  if (strcmp ( text, cmpstr )) {
      ...
  }

x.c:3: warning: pointer targets in initialization differ in signedness
x.c:6: warning: pointer targets in initialization differ in signedness
x.c: In function \rain':
x.c:10: warning: pointer targets in passing arg 1 of \x13trcmp' differ in signedness
x.c:10: warning: pointer targets in passing arg 2 of \x13trcmp' differ in signedness

The really strange thing is that strcmp() works with unsigned char inside:

#include <stdio.h>
#include <string.h>

const unsigned char*  text   = "Erdbärtörtchen";
const unsigned char*  cmpstr = "Eichhörnchenpfötchen";

int  main ( void )
{
    int  cmp;
    if (0 == strcmp ( text, cmpstr )) {
        fprintf (stderr, "Bug!\n");
    }
    cmp = strcmp ("Hase", "Häsin");
    if (cmp < 0)
        fprintf (stderr, "'Hase' < 'Häsin', i.e. compares 'unsigned char*'\n");
    if (cmp > 0)
        fprintf (stderr, "'Hase' > 'Häsin', i.e. compares 'signed char*'\n");
    return 0;
}

x.c:4: warning: pointer targets in initialization differ in signedness
x.c:5: warning: pointer targets in initialization differ in signedness
x.c: In function \rain':
x.c:10: warning: pointer targets in passing arg 1 of \x13trcmp' differ in signedness
x.c:10: warning: pointer targets in passing arg 2 of \x13trcmp' differ in signedness

---------------------------------------------------------------------------------
With C you only need to ignore the warnings, with C++ it becomes a painful.
Normally I work in the way only using 'signed char' and casting en masse.
National languages and C strings is something ...

---------------------------------------------------------------------------------

Frank Klemm

Re: Relaxing -Wsign-compare

[Gabriel Paubert]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

On Thu, 20 Sep 2001, Frank Klemm wrote:

> The main problem I have with signed vs. unsigned are normal strings.
> In Germany we have so called modified vowels, also other languages have
> modified letters. These are part of ISO-646 aka ISO-8859-1. But it is
> necessary to use 'unsigned char' for characters (otherwise you tap a rich
> pool of errors):
>
>    const unsigned char*  text = "Erdbärtörtchen";
>
> When you are using strcmp(), it complains about sign issue:

Did you ever hear about -funsigned-char ?

OFFTOPIC: what bothers me is that people compile all Linux/PPC for
example, vene the kernel, with -fsigned-char to silence warnings which
often point to actual bugs.

Like assigning the return value of fgetc() to a plain char and then
compared with EOF which correctly issues a message that the comparison
will always be false. And then somebody intervening to claim that the
right fix was compiling with -fsigned-char. I burst into flames directed
at the offender :-), but I digress, look at the Linux/PPC archives for
more fun.


	Gabriel (the only good char is the unsigned char).

Re: Relaxing -Wsign-compare

[Richard Henderson]

On Thu, Sep 20, 2001 at 03:45:27PM -0000, Morten Welinder wrote:
> Would it be reasonable to silence this warning when one of the sides is
> a constant (in the C meaning) that fits well within the range of the
> other side's type?

No.

C type rules still mandate the promotion of the signed argument to
unsigned.  Which means that if you do not know for a fact that the
signed argument is non-negative, you still have a problem.  Which
you can't know without data-flow analysis, and the warning is based
simply on the existance of the promotion.

Make 'i' a size_t like it ought to have been in the first place.


r~

Re: Relaxing -Wsign-compare

[Morten Welinder]

   On Thu, Sep 20, 2001 at 03:45:27PM -0000, Morten Welinder wrote:
   > Would it be reasonable to silence this warning when one of the sides is
   > a constant (in the C meaning) that fits well within the range of the
   > other side's type?

   No.

Dang.

   C type rules still mandate the promotion of the signed argument to
   unsigned.  Which means that if you do not know for a fact that the
   signed argument is non-negative, you still have a problem.  Which
   you can't know without data-flow analysis, and the warning is based
   simply on the existance of the promotion.

Ok, thanks.

   Make 'i' a size_t like it ought to have been in the first place.

That's a very popular non-solution.  Normally the index value is
actually used, like for example:

for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i++)
  printf ("%d\n", i);

With "int", the program is perfectly fine and portable.  With "size_t",
it isn't, if I understand things right.  There's no guarantee that "%d"
matches size_t.  Having to cast i when used is not appealing.

I guess an isizeof macro is my solution.

Thanks all!

Morten

Re: Relaxing -Wsign-compare

[Richard Henderson]

On Fri, Sep 21, 2001 at 05:48:31PM -0000, Morten Welinder wrote:
> With "int", the program is perfectly fine and portable.  With "size_t",
> it isn't, if I understand things right.  There's no guarantee that "%d"
> matches size_t.

So use %z, which does match size_t.


r~

Re: Relaxing -Wsign-compare

[Fergus Henderson]

On 21-Sep-2001, Richard Henderson <rth@redhat.com> wrote:
> On Fri, Sep 21, 2001 at 05:48:31PM -0000, Morten Welinder wrote:
> > With "int", the program is perfectly fine and portable.  With "size_t",
> > it isn't, if I understand things right.  There's no guarantee that "%d"
> > matches size_t.
> 
> So use %z, which does match size_t.

That's not portable.

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: < http://www.cs.mu.oz.au/~fjh >  |     -- the last words of T. S. Garp.

Re: Relaxing -Wsign-compare

[Frank Klemm]

On Fri, Sep 21, 2001 at 05:48:31PM -0000, Morten Welinder wrote:
> 
> That's a very popular non-solution.  Normally the index value is
> actually used, like for example:
> 
> for (i = 0; i < sizeof (foo) / sizeof (foo[0]); i++)
>   printf ("%d\n", i);
> 
> With "int", the program is perfectly fine and portable.  With "size_t",
> it isn't, if I understand things right.  There's no guarantee that "%d"
> matches size_t.  Having to cast i when used is not appealing.
> 
With 'int' the program is not portable if the array is equal or larger than
32 KByte. Example is Turbo-C for Atari ST/TT.

You have also problems with Arrays equal and larger than 2 GByte on every 64
bit machine.

-- 
Frank Klemm

Re: Relaxing -Wsign-compare

[Fergus Henderson]

On 20-Sep-2001, Richard Henderson <rth@redhat.com> wrote:
> 
> Make 'i' a size_t like it ought to have been in the first place.

Using unsigned types for loop indices is error-prone,
because it's easy to make mistakes like this one:

	size_t i;
	...
	for (i = n - 1; i >= 0; i--) {
		...
	}

Good compilers such as gcc will warn about such mistakes,
but nevertheless I think the danger of accidental wrap-around
causing problems like this is still a good argument for
preferring signed types for loop indices, except for cases where
there is a specific reason why the type needs to be unsigned.

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: < http://www.cs.mu.oz.au/~fjh >  |     -- the last words of T. S. Garp.