Re: [HACKERS] Fix warnings and typo in dshash
On Sun, Sep 3, 2017 at 2:56 PM, Thomas Munrowrote: > On Sun, Sep 3, 2017 at 6:57 PM, Amit Kapila wrote: >> I am seeing below warnings (on Win7) in dshash.c >> >> 1> dshash.c >> 1>src/backend/lib/dshash.c(318): warning C4334: '<<' : result of >> 32-bit shift implicitly converted to 64 bits (was 64-bit shift >> intended?) >> 1>src/backend/lib/dshash.c(679): warning C4334: '<<' : result of >> 32-bit shift implicitly converted to 64 bits (was 64-bit shift >> intended?) >> 1>src/backend/lib/dshash.c(713): warning C4334: '<<' : result of >> 32-bit shift implicitly converted to 64 bits (was 64-bit shift >> intended?) > > Thanks! That's a handy warning to have. I see that it is also > visible on the build farm: > > https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=caecilian=2017-09-02%2019%3A30%3A30=make > > Aside from these 3 warnings, it looks like the other 17 are all > "warning C4005: 'HAVE_LONG_LONG_INT_64': macro redefinition". I > wonder if it would make sense to fix that too and then turn on the > MSVC equivalent of -Werror=xxx on a build farm animal... > >> Attached a patch to fix the above warning. > > I think it should be (size_t) 1, not UINT64CONST(1). See attached. > Okay, that makes sense. Do you think we should also change type casting in BUCKETS_PER_PARTITION so that we are consistent? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix warnings and typo in dshash
Thomas Munrowrites: > Aside from these 3 warnings, it looks like the other 17 are all > "warning C4005: 'HAVE_LONG_LONG_INT_64': macro redefinition". Oh, I think that one might be my fault. I tweaked pg_config.h.win32 in 9d6b160d7 to use "#define HAVE_LONG_LONG_INT_64 1", for consistency with what happens in an autoconf'd build. But now I see that Solution.pm has another definition of that macro. (That sure looks like a mighty ad-hoc way of building ecpg_config.h, but whatever.) > I wonder if it would make sense to fix that too and then turn on the > MSVC equivalent of -Werror=xxx on a build farm animal... I don't have enough experience with MSVC to know if we want to commit to being 100% warning-free forevermore on it. But sure, somebody should try that on an experimental basis to see what happens. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix warnings and typo in dshash
Amit Kapilawrites: > I am seeing below warnings (on Win7) in dshash.c > 1> dshash.c > 1>src/backend/lib/dshash.c(318): warning C4334: '<<' : result of > 32-bit shift implicitly converted to 64 bits (was 64-bit shift > intended?) > 1>src/backend/lib/dshash.c(679): warning C4334: '<<' : result of > 32-bit shift implicitly converted to 64 bits (was 64-bit shift > intended?) > 1>src/backend/lib/dshash.c(713): warning C4334: '<<' : result of > 32-bit shift implicitly converted to 64 bits (was 64-bit shift > intended?) > Attached a patch to fix the above warning. That will just make for different warnings on 32-bit machines. Perhaps casting to size_t is appropriate here. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix warnings and typo in dshash
On Sun, Sep 3, 2017 at 6:57 PM, Amit Kapilawrote: > I am seeing below warnings (on Win7) in dshash.c > > 1> dshash.c > 1>src/backend/lib/dshash.c(318): warning C4334: '<<' : result of > 32-bit shift implicitly converted to 64 bits (was 64-bit shift > intended?) > 1>src/backend/lib/dshash.c(679): warning C4334: '<<' : result of > 32-bit shift implicitly converted to 64 bits (was 64-bit shift > intended?) > 1>src/backend/lib/dshash.c(713): warning C4334: '<<' : result of > 32-bit shift implicitly converted to 64 bits (was 64-bit shift > intended?) Thanks! That's a handy warning to have. I see that it is also visible on the build farm: https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=caecilian=2017-09-02%2019%3A30%3A30=make Aside from these 3 warnings, it looks like the other 17 are all "warning C4005: 'HAVE_LONG_LONG_INT_64': macro redefinition". I wonder if it would make sense to fix that too and then turn on the MSVC equivalent of -Werror=xxx on a build farm animal... > Attached a patch to fix the above warning. I think it should be (size_t) 1, not UINT64CONST(1). See attached. > I have noticed a typo in dshash.h for which a separate patch is attached. +1 -- Thomas Munro http://www.enterprisedb.com fix_warning.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fix warnings and typo in dshash
I am seeing below warnings (on Win7) in dshash.c 1> dshash.c 1>src/backend/lib/dshash.c(318): warning C4334: '<<' : result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) 1>src/backend/lib/dshash.c(679): warning C4334: '<<' : result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) 1>src/backend/lib/dshash.c(713): warning C4334: '<<' : result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) Attached a patch to fix the above warning. I have noticed a typo in dshash.h for which a separate patch is attached. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com fix_warnings_dshash_v1.patch Description: Binary data fix_typo_dshash_v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers