Обсуждение: some extra warnings from MSVC

Поиск
Список
Период
Сортировка

some extra warnings from MSVC

От
Peter Eisentraut
Дата:
I ran the MSVC build with the warning level set one level higher than 
the default (that is, meson -Dwarning_level=2, which actually maps to 
what MSVC considers its level 3), and it gave two new warnings that 
should be fixed:

../src/backend/utils/error/elog.c(1255): warning C4013: 'wchar2char' 
undefined; assuming extern returning int

../src/backend/utils/hash/dynahash.c(1767): warning C4334: '<<': result 
of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)

These are both in code that is new in PG19.

The first one is from commit 65707ed9afc (Add backtrace support for 
Windows).  This would be an error in gcc (from C99 on); it's kind of 
incredible that MSVC doesn't even warn about this by default.  I propose 
to add this warning category to the default set.

(Second thought: For consistency, make this an error, with '/we4013' 
instead of '/w24013'.)

The second one is from commit 13b935cd521 (Change dynahash.c and 
hsearch.h to use int64 instead of long).  I don't have a patch here to 
include this in the default warning set, mainly because it doesn't 
appear to map to any gcc warning option, but maybe we should add it 
anyway, since it can catch this kind of 4-byte-long-on-Windows issue.

Вложения

Re: some extra warnings from MSVC

От
Tom Lane
Дата:
Peter Eisentraut <peter@eisentraut.org> writes:
> The first one is from commit 65707ed9afc (Add backtrace support for 
> Windows).  This would be an error in gcc (from C99 on); it's kind of 
> incredible that MSVC doesn't even warn about this by default.  I propose 
> to add this warning category to the default set.
> (Second thought: For consistency, make this an error, with '/we4013' 
> instead of '/w24013'.)

+1 for making it an error.

> The second one is from commit 13b935cd521 (Change dynahash.c and 
> hsearch.h to use int64 instead of long).  I don't have a patch here to 
> include this in the default warning set, mainly because it doesn't 
> appear to map to any gcc warning option, but maybe we should add it 
> anyway, since it can catch this kind of 4-byte-long-on-Windows issue.

I think it'd be a good idea to warn even if we can't make gcc do that.
I think Windows is the only 64-bit platform we deal with where long
is just 32 bits, so covering the case in MSVC will expose bugs we
would not notice otherwise.

            regards, tom lane