Merge lp:~hjd/widelands/compilation-warnings into lp:widelands
Status: | Merged |
---|---|
Merged at revision: | 6480 |
Proposed branch: | lp:~hjd/widelands/compilation-warnings |
Merge into: | lp:widelands |
Diff against target: |
52 lines (+19/-9) 1 file modified
CMakeLists.txt (+19/-9) |
To merge this branch: | bzr merge lp:~hjd/widelands/compilation-warnings |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Widelands Developers | Pending | ||
Review via email: mp+141946@code.launchpad.net |
Description of the change
Related to the intial investigation of additional warnings in bug 1033615.
* WL_EXTRAWARNINGS doesn't exist anymore. If it is specified, cmake will warn it is an unused parameter. I have the impression very few people are using this, so I would rather move the warnings to the default configuration. I guess it was split out like this because -Wextra used to be extremely noise (several thousand extra lines).
* Warnings are grouped in three categories; generic, extra and gcc. Generic are the base warnings (-Wall -Wextra), extra adds additional warnings and gcc attempts to add warnings only availble in gcc.
Extra doesn't add a lot at the moment, but we might expand this in the future depending on the feedback on the bug report. Gcc has so far only added -Wlogical-op, mainly because it has already found a bug.
It is split this way because if an unknown warning is encountered, none of them is applied. This way, -Wall -Wextra should always be applied, if extra applies we get those warnings too and if it doesn't it will inform the user, and if the gcc warnings applies they are added too. In other words, if some of the extra warnings are not availble due to older compiler version or similar, we will always get a good base set of warnings. One thing I would like to see in the future is some way to check and add warnings individually so we get as many as possible. Maybe we could loop through the parameters sent to the compiler and only keep the ones supported.
This will be a gradual process though, as we figure out which warnings we want to add and which we want to silence. I think this is a good starting point though, and give us something we can build further upon.
I've rebuilt Widelands with both GCC and Clang with this revision, they both behave as expected and I got the same list of warnings as the previous run I uploaded to the warning bug reports.
PS. -Wno-attributes was removed because it didn't have any impact on the outcome. Were these warnings ignored due to false positives in the past?
A good idea i think. I merged this and have not seen any problems with building widelands so far. Testing for individual warnings will likely need a script to probe the compiler for what it supports.
-Wno-attributes was added by me IIRC because compilation on old gcc version made some noisy output because we use attribute hot in some places. My memory might fail me though, I am an old man after all.