Re: Revised patch for compare1.c test failures

[Zack Weinberg <zack at rabi dot columbia dot edu>]

On Mon, 20 Dec 1999 10:56:07 -0500 (EST), "Kaveh R. Ghazi" wrote:
> > From: Zack Weinberg <zack@rabi.columbia.edu>
> > 
> > Here is a revised and somewhat less invasive patch which corrects the
> > compare1.c test failures and some (not all) of the warnings in the
> > compiler itself.  Unfortunately, it creates a pile of new warnings
> > too, 37 total, almost all spurious.  It objects to code of the form
> > 
> >   expr ? (a = b) : (c = d);
> > 
> > where a is unsigned and c is signed.  In this context, the ? :
> > expression is being evaluated exclusively for its side effects and we
> > don't care that the signednesses don't match.  The warning is being
> > emitted from build_conditional_expr, where we don't know what the
> > context is.  It can be silenced by putting (void) on both branches,
> > but that is not K+R compatile...
> > 
> > Suggestions would be appreciated.
> > 
> > zw
> > 
> > 	* c-decl.c (finish_enum): Give the DECL_INITIAL of each enumerator
> > 	the type of the enumeration.
> > 	* c-typeck.c (build_conditional_expr): Warn for expr ? a : b where a
> > 	is signed and b is unsigned, or vice versa, if -Wsign-compare.
> > 	* testsuite/gcc.dg/compare1.c: Deobfuscate.  Test both
> > 	expr ? DImode : SImode and expr ? DImode : -1.
>
>
>So I tried it out and it does indeed remove the original spurious
>warnings and add the 37 new ones when you have mixed signed/unsigned
>in ?: branches.
>
>One question I have, should enums be treated as signed or unsigned?
>(I thought they were int by default, by then I'm no expert.)

It's implementation defined.  We make them signed iff they have negative
enumerators.  I can make a case for this or for using the integer literal
rules, but it may break something if we change it.

>Even if enums are really unsigned, isn't it possible to avoid warnings
>for something like this:
>
>expr ? 1U : -1
>
>because both 1U and -1 fit into an int?

I do avoid the warning for "expr ? 2U : 1".

I think we _want_ to warn about expr ? 1U : -1, because it _doesn't_ mean
expr ? 1 : -1.  It probably means expr ? 1U : UINT_MAX, but it might not (on
a non-two's-complement machine, which do exist, and I admit I'm not sure
what the standard says here).

>The enums would be a similar case if the max value of the enum was
>less than the max value of the other branch's type.  Maybe that's
>a way to elide the warning.

This would be better implemented by making enums signed if the signed type
can hold them, I think.

zw