MSVC: Improve warning options set

Поиск
Список
Период
Сортировка
От Peter Eisentraut
Тема MSVC: Improve warning options set
Дата
Msg-id bf060644-47ff-441b-97cf-c685d0827757@eisentraut.org
обсуждение исходный текст
Ответы Re: MSVC: Improve warning options set
Re: MSVC: Improve warning options set
Список pgsql-hackers
meson.build has a list of warnings to disable on MSVC:

   cflags_warn += [
     '/wd4018', # signed/unsigned mismatch
     '/wd4244', # conversion from 'type1' to 'type2', possible loss of data
     '/wd4273', # inconsistent DLL linkage
     '/wd4101', # unreferenced local variable
     '/wd4102', # unreferenced label
     '/wd4090', # different 'modifier' qualifiers
     '/wd4267', # conversion from 'size_t' to 'type', possible loss of data
   ]

First, these appear to be in some random order, so I wanted to sort 
them.  But then it also appeared that for some of these, if you remove 
the disablement, nothing changes, so it seemed some of these entries are 
not needed.

Some of these warnings are assigned to higher warning levels, so if 
someone wanted to compile PostgreSQL with a higher warning level on MSVC 
(similar to -Wextra), then disabling some from the higher levels is 
useful.  But I figured this could be explained better.

So what I did is sort these and group them by the warning level assigned 
by MSVC.

Actually, one of them (the "unreferenced label" one) was not enabled by 
default on any level, and conversely I think we do actually want that 
one enabled, since we use -Wunused-label on other compilers, so I 
changed the disablement to an enablement.

Then I also found a few more warnings that would be useful to enable. 
So I'm proposing to add warnings that are similar to -Wformat and -Wswitch.

Here are some documentation links:

https://learn.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=msvc-170
https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warnings-c4000-c5999?view=msvc-170

https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warnings-by-compiler-version?view=msvc-170


With that done, I also looked at the disabled warnings to see what could 
be done to fix the underlying issues.  In particular, I looked at

     '/wd4273', # inconsistent DLL linkage

If you enable that one, you get this:

../src/backend/utils/misc/ps_status.c(27): warning C4273: 
'__p__environ': inconsistent dll linkage
C:\Program Files (x86)\Windows 
Kits\10\include\10.0.22621.0\ucrt\stdlib.h(1158): note: see previous 
definition of '__p__environ'

The declaration in ps_status.c was:

     #if !defined(WIN32) || defined(_MSC_VER)
     extern char **environ;
     #endif

The declaration in the OS header file is:

     _DCRTIMP char***    __cdecl __p__environ (void);
     #define _environ  (*__p__environ())

So it is clear why a linker might be upset about this.

To fix this, we can just remove the || defined(_MSC_VER).

Note that these conditionals around the environ declarations were added 
only somewhat recently in commit 7bc9a8bdd2d ("Fix warnings about 
declaration of environ on MinGW.").

Maybe there are older versions of Windows where the declaration is 
needed, maybe this is a ucrt vs. msvcrt thing, don't know, testing is 
welcome.


The business in ps_status.c is kind of weird anyway, because we only 
need the environ variable in the PS_USE_CLOBBER_ARGV case, which is not 
Windows, so maybe we should move some things around to avoid the problem 
in this file.  But there are also a few other files where environ is 
declared for use by Windows as well.

Вложения

В списке pgsql-hackers по дате отправления: