Author Alexander Riccio
Recipients Alexander Riccio, Jim.Jewett, loewis, paul.moore, serhiy.storchaka, skrah, steve.dower, tim.golden, zach.ware
Date 2020-03-19.19:45:49
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1584647150.46.0.654769200748.issue25878@roundup.psfhosted.org>
In-reply-to
Content
Ok, so I finally have some proper time to work on this. How would people (who are higher up in python than me, obviously) feel about suppressing most of the warnings via a user macro in Visual Studio? I've found that it's quite easy to add a macro to the project properties (i.e. pyproject), and have it import into the "Disable specific warnings" build options. This keeps our build files hand-crafted (let's keep the hipsters happy!), and consistent. By doing this, I can get it down to ~100 warnings, which are essentially code that we may want to keep an eye on.

The following warnings are essentially always spurious for python. C4100 is
everywhere, and is nearly always benign, but would still require a lot of
careful inspection to determine it its truly on purpose. C4127 is fine since C
doesn't have `constexpr`, let alone `if constexpr`, and it's just easier to
use if statements than  `#ifdef`s everywhere. C4152 is used in every module
exports array. 

C4100 (unreferenced formal parameter) https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4100?view=vs-2019
C4115 'type' : named type definition in parentheses https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-levels-1-and-4-c4115?view=vs-2019
C4127 conditional expression is constant https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4127?view=vs-2019
C4131 'function' : uses old-style declarator https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4131?view=vs-2019
C4152 non standard extension, function/data ptr conversion in expression https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4152?view=vs-2019

These are usually spurious, since zero-sized arrays are common across the C
world, and is one of several ways of implementing dynamic structs (e.g. unsized
arrays, ANYSIZE_ARRAY, etc...). C4204 and C4221 exist all over the place, and
are probably fine as long as we don't make any lifetime mistakes, which of
course is a solved problem in C. Instances of C4244 should each be individually
inspected carefully since they could be bugs, but there are way too many of
them to allow the warning right now. 

C4200 nonstandard extension used : zero-sized array in struct/union https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-levels-2-and-4-c4200?view=vs-2019
C4201 nonstandard extension used : nameless struct/union https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4201?view=vs-2019
C4204 nonstandard extension used : non-constant aggregate initializer https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4204?view=vs-2019
C4221 nonstandard extension used : 'identifier' : cannot be initialized using address of automatic variable https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4221?view=vs-2019
C4232 nonstandard extension used : 'identifier' : address of dllimport 'dllimport' is not static, identity not guaranteed https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4232?view=vs-2019 
C4244 'conversion' conversion from 'type1' to 'type2', possible loss of data https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-levels-3-and-4-c4244?view=vs-2019
C4245 'conversion' : conversion from 'type1' to 'type2', signed/unsigned mismatch https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4245?view=vs-2019


Instances of C4389 should each be individually inspected carefully since they
could be bugs, but there are way too many of them to allow the warning right now.

C4389 'operator' : signed/unsigned mismatch https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4389?view=vs-2019


Instances of C4456 and C4457 are likely ok given the length of many CPython
functions, but should be inspected for mistakes. There are lots of places
where `i` and such are used as variables, and it's there's no way to know
if the author intended it.
C4456 declaration of 'identifier' hides previous local declaration https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4456?view=vs-2019
C4457 declaration of 'identifier' hides function parameter https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4457?view=vs-2019


Instances of C4701 occur pretty frequently around code that initializes C
objects with the funky PyArg_ParseTuple format string mechanism, since the
compiler has no built in support for understanding custom format strings.
Personally, I would support zero-initializing these variables. Would it break
anything? I assume there's a performance impact, but the compiler can't see
that across translation units. If there were some other way to enforce proper
variable initialization other than zeroing, that would work too. We could
ABSOLUTLEY use SAL for this to get the performance benefit of not zero
initializing everything and statically checking if the functions are being used
correctly, but I doubt there's community support for that. 4706 is addressed
already. 4702 is everywhere, and would require a lot of inspection to suppress the warning.

C4701 Potentially uninitialized local variable 'name' used https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4701?view=vs-2019
C4702 unreachable code https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4702?view=vs-2019
C4703 Potentially uninitialized local pointer variable 'name' used https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703?view=vs-2019
C4706 assignment within conditional expression https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4706?view=vs-2019


I could submit a patch to set up the macro with a few warnings that we can all
agree on, and not turn on W4 yet, or I can submit a patch with those few
warnings disabled and W4 enabled but very noisy builds. What is preferred?

List formatted for macro:
4100;4115;4127;4131;4152;4200;4201;4204;4221;4232;4244;4245;4389;4456;4457;4701;4702;4703;4706
History
Date User Action Args
2020-03-19 19:45:50Alexander Ricciosetrecipients: + Alexander Riccio, loewis, paul.moore, tim.golden, skrah, Jim.Jewett, zach.ware, serhiy.storchaka, steve.dower
2020-03-19 19:45:50Alexander Ricciosetmessageid: <1584647150.46.0.654769200748.issue25878@roundup.psfhosted.org>
2020-03-19 19:45:50Alexander Ricciolinkissue25878 messages
2020-03-19 19:45:49Alexander Ricciocreate