[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: Ok, so a draft of this produces 34 warnings, but makes way more changes to the .vcxproj and .filters files than I think it should: https://github.com/ariccio/cpython/commit/60152aa065a3ad861f0359a8ada7f2fbc83a3933 Before I submit a PR, I think I should

[issue25878] CPython on Windows builds with /W3, not /W4

Steve Dower added the comment: Thanks for all that work! We definitely don't want a noisy build to be merged, but if you submit a PR with only your most confident suppressions then it'll be easier for us to look at the others and see which are scary/not-scary. --

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: 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

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: One more thing, after I ran code analysis: This is obviously a potential memory leak: Warning C6308 'realloc' might return null pointer: assigning null pointer to 'arr->items', which is passed as an argument to 'realloc', will cause the original memory

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: I decided to come back to this after a python meetup last night. By messing with this a bit, building in VS2019 with /W4, I see that fully 2/3rds of the total warnings are from two specific warnings: C4100 (unreferenced formal parameter) C4127

[issue25878] CPython on Windows builds with /W3, not /W4

Jim Jewett added the comment: Is there a way to document why certain warnings are being suppressed, in case someone wants to revisit the suppression later? -- nosy: +Jim.Jewett ___ Python tracker

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: If there are few enough instances, then using a #pragma warning(suppress:4232) is probably the best idea. -- ___ Python tracker

[issue25878] CPython on Windows builds with /W3, not /W4

Stefan Krah added the comment: I would expect linking to behave "as if" the abstract machine defined by the standard were executing the code. We already had one function pointer equality issue due to COMDAT folding, so I agree that we should leave the warning on. --

[issue25878] CPython on Windows builds with /W3, not /W4

Stefan Krah added the comment: Pragma added for libmpdec in #26139. -- ___ Python tracker ___ ___

[issue25878] CPython on Windows builds with /W3, not /W4

Steve Dower added the comment: libmpdec/memory.c keeps pointers to customisable memory functions (malloc/free) and initialises them to and These functions are dllimport'd from the CRT, and so they trigger a warning letting you know that " == " may not always be true. (That trivial

[issue25878] CPython on Windows builds with /W3, not /W4

Steve Dower added the comment: I made a new patch to replace all of the existing ones here with the two-line version of the change. I also suppressed warning 4232 about dllimport addresses (essentially, you may get different values when taking the address of an imported function because of

[issue25878] CPython on Windows builds with /W3, not /W4

Stefan Krah added the comment: Ah, thanks! I guess this Visual Studio behavior is not compatible with the C-standard, but indeed there are no function pointer comparisons in libmpdec. -- ___ Python tracker

[issue25878] CPython on Windows builds with /W3, not /W4

Stefan Krah added the comment: Could you explain warning #4232 in the case of libmpdec? I thought all files are compiled and linked together. Why is the function imported? -- nosy: +skrah ___ Python tracker

[issue25878] CPython on Windows builds with /W3, not /W4

Steve Dower added the comment: I don't think the C standard necessarily applies or defines what happens when you call/link a __declspec'd function, but it's certainly not going to be the same as static linking. That said, maybe the subtle nature of this means we should suppress the warning

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: > This should be about a 2 line change, but the current patch is several > hundred lines of spam. I agree, but wasn't immediately sure how to do so. Unfortunately, I've been working on other things, and I'm not sure when I'll be able to finish this.

[issue25878] CPython on Windows builds with /W3, not /W4

Zachary Ware added the comment: That's alright, it'll be here whenever you come back to it, unless somebody else takes it up :) By the way, my wording was poor in my previous message. My 'hundreds of lines of spam' remark was not meant to disparage your patch, but rather Visual Studio's

[issue25878] CPython on Windows builds with /W3, not /W4

Serhiy Storchaka added the comment: Thank you Alexander for your work. I agree that we have to use maximal warning level available in the compiler, and specially disable those warning categories that produces mainly false positives with CPython sources. See also similar issue23545 about GCC.

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: > The problem with this bug report is that there is little chance that it gets > resolved in the near term, and it's quite possible that it will stay open for > years. Somebody would have to sit down and start producing patches to fix > these warnings

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: Cut out more noisy warnings. -- Added file: http://bugs.python.org/file41333/W4_v3.patch ___ Python tracker ___

[issue25878] CPython on Windows builds with /W3, not /W4

Zachary Ware added the comment: At this point our MSBuild project files are hand-crafted, and are best edited by hand. You can use the GUI to get a general idea of what the changes should be, but it just creates far too much spam to really be usable. This should be about a 2 line change, but

[issue25878] CPython on Windows builds with /W3, not /W4

Martin v. Löwis added the comment: The problem with this bug report is that there is little chance that it gets resolved in the near term, and it's quite possible that it will stay open for years. Somebody would have to sit down and start producing patches to fix these warnings correctly,

[issue25878] CPython on Windows builds with /W3, not /W4

New submission from Alexander Riccio: This issue is related to Issue25847. Compiling at /W4 is generally a good idea. It's an industry best practice, and even though I don't expect disagreement, I'll throw in a few coding standard links:

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: The warnings that I've disabled are: C4054, "'conversion' : from function pointer 'type1' to data pointer 'type2'": https://msdn.microsoft.com/en-us/library/07d15ax5(v=vs.90).aspx I disabled 4054because there are lots of void* to (somefuncptr)

[issue25878] CPython on Windows builds with /W3, not /W4

Alexander Riccio added the comment: I've added the text build output. -- Added file: http://bugs.python.org/file41322/W4_v2_build_output ___ Python tracker

[issue25878] CPython on Windows builds with /W3, not /W4

Changes by Alexander Riccio : -- nosy: +paul.moore, steve.dower, tim.golden, zach.ware ___ Python tracker ___

[issue25878] CPython on Windows builds with /W3, not /W4

Changes by Alexander Riccio : -- components: +Windows ___ Python tracker ___ ___