Michael Paquier <michael@paquier.xyz> writes:
> On Tue, May 01, 2018 at 05:40:18PM +1200, Thomas Munro wrote:
>> src/backend/replication/basebackup.c(1470): warning C4146: unary minus
>> operator applied to unsigned type, result still unsigned
>> ... where cnt is size_t. Perhaps we should use (or cast to) off_t?
> Sounds sensible.
+1.
>> We have: uint64 result = seed ^ (sizeof(int64) * MM2_MUL);
>>
>> ... where MM2_MUL is a UINT64CONST. I checked the upstream source of
>> this code and it's using a runtime multiplicand while here it's a
>> constant so the compiler sees the overflow. I suppose we could make
>> the warning go away by just defining a constant (which I make out to
>> be 0x35253c9ade8f4ca8).
> Or just enforce it with some casts?
I'm not sure we could silence the warning with casts. Thomas' proposal
of a hand-computed constant seems reasonable.
>> C:\Program Files (x86)\Microsoft Visual Studio
>> 12.0\VC\include\stdbool.h(11): warning C4005: 'false' : macro
>> redefinition [C:\buildfarm\buildenv\HEAD\pgsql.build\jsonb_plperl.vcxproj]
>> c:\buildfarm\buildenv\head\pgsql.build\src\include\c.h(283)
>> : see previous definition of 'false'
>> C:\Program Files (x86)\Microsoft Visual Studio
>> 12.0\VC\include\stdbool.h(12): warning C4005: 'true' : macro
>> redefinition [C:\buildfarm\buildenv\HEAD\pgsql.build\jsonb_plperl.vcxproj]
>> c:\buildfarm\buildenv\head\pgsql.build\src\include\c.h(279)
>> : see previous definition of 'true'
> Those are caused by the interactions of stdbool.h from MSVC and the
> definitions from c.h.
Yeah. In the wake of Peter's changes to use <stdbool.h> on other
platforms, should we be enabling HAVE_STDBOOL_H for Windows?
On more or less the same topic, I just scraped all the compiler warnings
for HEAD from the buildfarm database, and there seem to be a few other
things worth cleaning up. One that I'm looking at is that recent gcc
has a -Wimplicit-fallthrough warning for switch branches not separated
by a "break" or similar. It can be silenced with a comment similar to
/* FALLTHROUGH */, but we have not been entirely consistent about
providing such comments. I'm inclined to run around and fix those
omissions. Perhaps at some point we should have configure turn that
warning on if available?
regards, tom lane
