[cfe-dev] Possibly invalid enum tautology warning

[Peter Geoghegan]

On 7 August 2011 22:21, John McCall <rjmccall at apple.com> wrote:
> On Aug 7, 2011, at 1:32 PM, David Blaikie wrote:
>> On Sun, Aug 7, 2011 at 12:29 PM, John McCall <rjmccall at apple.com> wrote:
>>> but it sounds
>>> like you're arguing that we should suppress warnings when another
>>> platform exists where the warning wouldn't trigger.
>>
>> In this particular case I think that's what he's saying, yes. But I
>> wouldn't say that implies the general statement you've made.

That isn't what I meant at all. I don't like tautologies any more than
you guys. What I meant was that there could in principle be a compiler
informed by exactly the same concerns as Clang and exactly as mature,
that differed only in that it invariably made enums signed, which is a
very small difference that I believe is allowed for by the standard.
That compiler wouldn't show this warning though, because it wouldn't
be tautological there merely because, as it happens, enums are never
unsigned there. /The tautology is not essential to the code/.

> 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:

"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."

It seems to me that C99 isn't encouraging or requiring this sort of
promotion at all. Maybe this has been widely interpreted to mean "you
should promote". I don't know - I'm not that much of a language
lawyer. There doesn't seem to be anything in this passage to suggests
that promotion should or must happen though.

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.

>>> it's
>>> contrary to what most people writing portable code want;  indeed,
>>> we frequently get the dual request, to enable warnings on code that
>>> works on the current platform but wouldn't on something else.
>>
>> & what's your opinion when such a request is made?
>
> That it's an interesting idea, but extremely false-positive prone, and it
> would be expensive to get most of those false positives out.  Very few
> people are interested in truly arbitrary portability — I mean, case in
> point, our code base consistently assumes that 'unsigned' is at least
> 32 bits (the host unsigned, not the target unsigned, of course).  That's
> not a perfectly portable assumption at all, but it's true of every
> platform we will ever care about running on.  There's no good way
> to deduce that kind of thing.

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.

> 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.

> I think a good portability checker might make an interesting
> static analysis, though.

Sure.

On 7 August 2011 19:46, David Blaikie <dblaikie at gmail.com> wrote:
> Did you blog about this particular issue? I'd be curious to see what
> construct it found.

I referred to it in passing in my most recent blog post. Here's the
relevant commit:

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=05e83968929f4ec1eba058fcae755fd2df98864e

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services