-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Treat all warnings as errors #685
Comments
this should be toggleable - introduce |
You'll run into problems with that, because whenever you include a 3rdparty header you get all of its warnings. So it's not as easy as just enabling WX because you don't want to be bogged down into forking every dependency and "fixing" their warnings. P.s. there was a story one about an intern or a junior who fixed uninitialised variables in openssl and broke it cause they were actually a source of entropy for the rand gen |
I would limit fixing them to our headers & sources only |
yeah I get you, though I don't think that would be a case if this happened in our codebase, I rather stick to initialize variables not to some random values which may lead to bugs, but of course it's not a case as well to fix all 3rdparty headers |
well another problem this could introduce an implicit vendor/toolset dependency, but since we are targeting latest toolsets and cpp standard I would not care much |
by the way, there are solutions how to disable/ignore warnings in external headers coming from 3rdparty and still treat all warnings as errors with our codebase
also https://devblogs.microsoft.com/cppblog/broken-warnings-theory/ |
with that you don't have this problem you mentioned
|
Nabla should be compiled with
/WX
(for MSVC, for GCC/Clang-Werror
) flag enabled for compiler and linker which treats all warnings as errors, then all of them should be eliminated and sources adjusted. Doing so may save us in future, recursive warning generation may lead to unresponsive builds taking more then half an hour, also we decrease a chance for undefined behaviours at runtime.The text was updated successfully, but these errors were encountered: