Treat warnings as errors on Clang and GCC#6759
Conversation
There was a problem hiding this comment.
-Wshorten-64-to-32 needs to be treated as a warning, or maybe even suppressed - it's enabled by default on Apple Clang 14 and throws tons of warnings, which are now errors and prevent building.
Also a bit of nitpicking, I feel like commit messages could be improved. The Fix compilation errors are only errors after warnings are turned into errors - it seems more correct to me to have the "Treat warnings as errors" at the end and then have other fixes as Fix compilation warnings.
First (or last, depending on where you put it in the end) commit needs cmake: prefix.
Thanks I only used the CI to test the PR, so I didn't know about how it behave with Xcode 14.
I fixed the errors afterward, some didn't appeared before |
167085f to
c718677
Compare
57f4e50 to
8cb25bb
Compare
WizardCM
added
the
kind/cleanup
8cb25bb to
0a9d2e5
Compare
|
Apart from open inline comments I'm fine with this. |
5ac157c to
e1c2313
Compare
tytan652
changed the title
564b123 to
86b6167
Compare
86b6167 to
3eb13f5
Compare
|
Changed some CMake conditions to apply Clang/GCC flags with |
3eb13f5 to
30ed9b0
Compare
|
Did my best for making the commit name shorter. |
|
Reminder that there are merge conflicts that need to be resolved. |
|
Overall seems fine to me - will we merge the separate PRs instead of this "meta" PR? Because as-is the changes lack context (no code comments referencing the warning that is silenced), while separate commits would give some needed context. |
No, this is one PR, you should have asked that really sooner.
21 commits with descriptions when needed. And you say there is no context. |
If the commits are retained that should be good enough then. Didn't know whether it was planned to squash them. |
- Fix Python deprecation warnings - Fix maybe uninitialized warning (GCC only)
- Fix format warning On Linux uint64_t is a unsigned long and on macOS it is a unsigned long long. - Fix macro-redefine warning on macOS
- Fix format warnings - Fix unused-parameter warnings
Only with SWIG version earlier than 4.1 because it is fixed upstream. swig/swig@fa2f9dc
-Wformat-security is enabled, some Linux packages build system have it enabled. obsproject#5766 -Wunused-parameter is enabled too. Also ensure that null conversion (C++ only for GCC) is enabled because its enablement by default can depend on the CMake generator. -Wswitch is enabled but it is kept as a warning. It was already enabled with Clang. If Clang has -Wshorten-64-to-32 enabled, keep it as warnings.
GCC on aarch64 seems to found type-limits issue in the code where GCC x86_64 does not.
Calm libobs-opengl deprecation warnings on macOS. Calm FFmpeg deprecation warning with LIBAVFORMAT_VERSION_MAJOR < 59.
Those warnings appeared with GCC 12 with -O2, those are potentially created by regression from GCC.
-Wenum-conversion is only available with GCC from the version 10.
Freedesktop Flatpaks 22.08 provide this version of GCC.
8 participants
Description
Depends on:
Fix macro-redefine warning with Qt loop enabled obs-browser#368Switch clean up (remove one-case switch and switch warning workaround) #6960obs-websocketfine with Clang on LinuxOnce those are merged, commits related to those PRs will be removed.
There is no longer commits from other PRs on this PR.The scope of the PR is do the same as #6723 but for Clang and GCC and fix (if possible) actual warnings that were found from CI, Arch Linux (my machine) and Xcode 14.
Please test this PR on your Linux env with Release, RelWithDebInfo and Debug (GCC and clang).
Example of why:
Turn every warning into errors except:
-Wformat-security is also enabled to avoid PR like #5766 in the future.
All the errors made by this change are fixed in this PR.
Since between some generators there is different enablement (e.g here between Ninja and Xcode) I ensured that null conversion warning on Clang and GCC is enabled.
Motivation and Context
Do the same as #6723.
How Has This Been Tested?
Types of changes
Checklist: