Re: Further news on Clang - spurious warnings

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Peter Geoghegan <peter(at)2ndquadrant(dot)com>
Cc: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Further news on Clang - spurious warnings
Date: 2011-08-03 11:19:29
Message-ID: 4E392EC1.2060504@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 03.08.2011 14:13, Peter Geoghegan wrote:
> On 3 August 2011 11:05, Peter Geoghegan<peter(at)2ndquadrant(dot)com> wrote:
>> I don't believe that the standard allows for an implementation of
>> enums as unsigned integers - after all, individual enum literals can
>> be given corresponding negative integer values.
>
> It actually gives leeway to implement the enum as unsigned int when
> the compiler knows that it won't matter, because there are no negative
> integer values that correspond to some enum literal. The hint was in
> my original warning. :-)
>
>> This warning is only seen because the first enum literal in the enum
>> is explicitly given the value 0, thus precluding the possibility of
>> the value being< 0, barring some abuse of the enum.
>
> It's also seen if no explicit values are given, and the compiler opts
> to make the representation unsigned. It is not seen if it the value is
> -1, for example.
>
> Despite the fact that whether or not the value is unsigned is
> implementation defined, I think that the patch is still valid - the
> expression is at least logically tautological, even if it isn't
> necessarily bitwise tautological, because, as I said, barring some
> violation of the enum's contract, it should not be< 0. That's
> precisely why the compiler has opted to make it unsigned.

Right, but the purpose of that check is to defend from programmer error.
If the programmer screws up and calls "PQresStatus(-1)", we want to give
an error, not crash. If you assume that the programmer will only pass a
valid enum constant as parameter, then you might as well remove the
if-statement altogether. I don't think that would be an improvement.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2011-08-03 11:44:40 Re: Further news on Clang - spurious warnings
Previous Message Peter Geoghegan 2011-08-03 11:13:20 Re: Further news on Clang - spurious warnings