[cfe-dev] Possibly invalid enum tautology warning

[John McCall]

On Aug 7, 2011, at 4:16 PM, Peter Geoghegan wrote:
> On 7 August 2011 22:21, John McCall <rjmccall at apple.com> wrote:
>> He phrased it pretty generally.  I was shying away from the specifics
>> of this case because, while the underlying type of an enum is
>> indeed implementation-specific, the promotion rules actually are
>> governed by the standard, and therefore the behavior of this
>> promotion is not really compiler-specific at all.  This would be
>> clearer if we saw the actual enum definition.
> 
> Are you sure that the promotion rules are governed by the standard?
> Where, exactly? This is from C99:

Ah.  It does depend which language we're talking about here.

> "Each enumerated type shall be compatible with char, a signed integer
> type, or an
> unsigned integer type. The choice of type is
> implementation-defined,110) but shall be
> capable of representing the values of all the members of the enumeration."

C99 6.3.1.8:
  Many operators that expect operands of arithmetic type cause conversions
  and yield result types in a similar way. The purpose is to determine a
  common real type for the operands and result.
  ...
  [If neither operand is long double, double, or float], the integer promotions
  are performed on both operands.
C99 6.3.1.1:
  The following may be used in an expression wherever an int or unsigned
  int may be used:
    — An object or expression with an integer type whose integer conversion
      rank is less  than or equal to the rank of int and unsigned int.
    — A bit-field of type _Bool, int, signed int, or unsigned int.
  If an int can represent all values of the original type, the value is converted
  to an int; otherwise, it is converted to an unsigned int. These are called the
  integer promotions.

Clang follows GCC's lead in making unpacked C enums compatible with
unsigned unless they contain signed enumerators;  the enum type then
has the integer rank of 'unsigned int' for the purposes of these rules.

So promotion is indeed mandated, but whether it promotes to 'int' or
'unsigned int' is implementation-defined.

The C++ rule is quite different:
  C++0x [conv.prom]p3:
    A prvalue of an unscoped enumeration type whose underlying type
    is not fixed (7.2) can be converted to a prvalue of the first of the following
    types that can represent all the values of the enumeration (i.e., the values
    in the range bmin to bmax as described in 7.2): int, unsigned int, long int,
    unsigned long int, long long int, or unsigned long long int.

> I attached my example, but I guess that isn't supported on this list.
> Here it is again:
> 
> #include <stdio.h>
> 
> typedef
> enum tester
> {
> 	test_constant = 0,	
> 	other_test_constant
> } tester;
> 
> void show_tautology(tester test);
> 
> int main()
> {
> 	tester test = test_constant;
> 	show_tautology(test);
> 	return 0;
> }
> 
> void show_tautology(tester test)
> {
> 	if (test < 0)
> 		printf("is the above test necessarily tautological?");
> }
> 
> I'm simply building with clang (not clang++) from SVN tip.

Okay, yes, in C, when we're emulating the GCC behavior, this
comparison is performed in the type 'unsigned int' and is therefore
tautological.

I still consider this a useful warning, even on enum types.  When I
was implementing it, I was persuaded that we ought to have a
special case for comparing against an enumerator which happens
to be zero, because that was indicative of a particular common
and harmless idiom (specifically, things like if (v < v_first || v > v_last)).
I'm loathe to extend that to a direct comparison against zero, where
it really does seem like the programmer is specifically testing for
an error case and might be surprised to realize that their test never
triggers.  People who really do want to litter their code with defensive
checks like this should just disable the tautological comparison
check — it's not likely that it would only ever come up with enums.

> Maybe so. I'm not sure that applies in this case, because I'm not sure
> if some /relevant/ compiler exists that actually never has unsigned
> enums. It's certainly possible, as I've said. I certainly wouldn't
> advocate spending a lot of time on it. I just thought that it was
> worth drawing the community's attention to this issue.

I do appreciate that.

>> It's hard to get too excited about adding
>> extensive portability checking when any sane setup will have a
>> buildbot which warns about the problem within a couple of hours.
> 
> The issue here is that a warning is seen which is valid for Clang, but
> may not necessarily be valid on other platforms, entirely because of a
> reasonable design decision that the compiler author made that is
> totally unrelated to the quality of the code that is being compiled. A
> buildbot isn't going to help here.

Well, you're assuming that comparisons like this are always in overly-
defensive code where the tautology is harmless.

John.