eliminate or suppress various header warnings under MSVC with /W4 #21031
Conversation
Building with /W4 on MSVC would produce a warning like: inline.h(309): warning C4310: cast truncates constant value While the casting here is harmless, it's reasonable to eliminate warnings caused by headers that embedders or extension builders might use. Fixes Perl#20999
Setting si_cxsubix in the context helper functions from the difference of two pointers on 64-bit builds produced warnings on MSVC like: inline.h(2815): warning C4244: '=': conversion from '__int64' to 'I32', possible loss of data Similar warnings are produced on gcc with -Wconversion (and many more) The machine that needs 2**32 context entries would also require much more memory to store the contexts themselves and any SVs and stack entries needed within each context, so I've cast the values rather than making si_cxsubix a SSize_t. Fixes parts of Perl#20841, preventing this warning when the header is used by embedders or extension builders.
These functions are also used as pointers, so we need to retain the context parameter to be compatible with the other similar functions that do use their context.
win32/win32.h
Outdated
| @@ -274,7 +274,7 @@ typedef struct | |||
| { | |||
| FILE _public_file; | |||
| char* _ptr; | |||
| }; | |||
| } _u; | |||
There was a problem hiding this comment.
This is not the commit to fix this, but it is undefined behavior for any symbol to begin with an underscore. So, in theory all the similar symbols should be changed to like u_, which likely will never happen.
There was a problem hiding this comment.
"All identifiers that begin with an underscore are always reserved for use as identifiers
with file scope in both the ordinary and tag name spaces."
This means they're reserved a names at file scope (variables, functions, typedefs at file scope) and tag names, like struct this, union that or enum andthis.
They aren't reserved universally like names starting with underscore followed by an uppercase letter or starting with two underscores. (Or any name containing two underscores in a row in C++.)
But I've changed it for the new name anyway.
This warned on MSVC
MSVC complains about truncation (it isn't truncation) when casting the pointer to bool.
This warned on MSVC: win32.h(277): warning C4201: nonstandard extension used: nameless struct/union
MSVC complained about this code: cop.h(34): warning C4324: 'jmpenv': structure was padded due to alignment specifier which happens due to the jmpbuf structure in MSVC having an alignment attribute set.
tonycoz
force-pushed
the
20999-mask-utf-is-continuation-mask
branch
from
6817be5
to
194aa29
Compare
I wasn't sure myself which was best here, I've changed them to |
These prevents noise for downstream embedders. PR #21031
|
Applied manually. |
As illustrated by #20999 it's common for projects to be built with the maximum warnings, and it's a common recommendation.
It's also something we follow with gcc, g++ and clang on POSIX-ish systems.
This change allows embedders of perl using MSVC to enable /W4 and include the perl headers, eliminating most warnings.
With these changes I get no warnings from the headers building
..\toke.objwith/W4, which is at least a good start.I haven't tried to eliminate all warnings in the perl build, that would be a much larger change.
https://github.com/cpp-best-practices/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#compilers
https://www.learncpp.com/cpp-tutorial/configuring-your-compiler-warning-and-error-levels/
https://www.reddit.com/r/programming/comments/1m263m/turn_on_your_damn_warnings/