suppress msvc warnings about totally safe functions#298
suppress msvc warnings about totally safe functions#298danmar merged 2 commits intocppcheck-opensource:masterfrom diamante0018:master
Conversation
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
Sounds good to me. I am no cmake expert so I can't say anything about that change.
Adding such pragmas in the header is fine for me.
But if ci is happy I think we can merge this.
|
CI is all green first try? Impossible 😺 |
|
"totally safe" is an overstatement. The warnings definitely indicate issues but fixing these kind of warnings has been a sore spot for a while so let's not get into this. |
| if (CMAKE_CXX_COMPILER_ID MATCHES "GNU") | ||
| add_compile_options(-Wall -Wextra -pedantic -Wcast-qual -Wfloat-equal -Wmissing-declarations -Wmissing-format-attribute -Wredundant-decls -Wshadow -Wundef -Wold-style-cast -Wno-multichar) | ||
| elseif (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") | ||
| add_compile_definitions(_CRT_SECURE_NO_WARNINGS) |
There was a problem hiding this comment.
These warnings should be "fixed" by specifying _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES=1 instead - see https://learn.microsoft.com/en-us/cpp/c-runtime-library/secure-template-overloads.
I wasn't aware of this so it might have been introduced only in recent years.
There was a problem hiding this comment.
I'm not so sure about this, it seems to be the usual MSVC scaring people into using _s functions when the 'normal' C standard functions can be used just fine if used correctly. _CRT_SECURE_NO_WARNINGS is just the broader macro that tells MSVC to stop complaining.
|
|
||
| #if defined(_MSC_VER) | ||
| // suppress warnings about "conversion from 'type1' to 'type2', possible loss of data" | ||
| # pragma warning(disable : 4267) |
There was a problem hiding this comment.
This is really, really, really, REALLY bad as it will disable the warnings for every file which includes this. Use add_compile_options() with the /Wd Visual Studio options instead.
There was a problem hiding this comment.
Yeah you are right I should have used pragma push(warning) and pop surrounding this header file but I forgot
3 participants
Compiling with MSVC proves to be an unpleasant experience due to MSVC unreasonably complaining about the following:
It complains about this library using
fopeninstead of their CRT non-standardfopen_sfunction.This warning cannot be suppressed by adding a definition to the
simplecpp.hheader, it does not work somehow.It is a bug I've always had with programs compiled with MSVC. I have always added
_CRT_SECURE_NO_WARNINGSto the preached header but because this repository does not have one I have opted to add it to the CMAKE file.Finally, the most annoying warnings are suppressed successfully using pragma directives.
Without these pragmas, my MSVC produces an immense build log full of warnings.
I hope this PR is appreciated in respect of: #288 (comment)