Hans Joachim Desserud has proposed merging
lp:~hjd/widelands/compilation-warnings into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1033615 in widelands: "Consider checking for more warnings when
compiling Widelands with WL_EXTRAWARNINGS"
https://bugs.launchpad.net/widelands/+bug/1033615
For more details, see:
https://code.launchpad.net/~hjd/widelands/compilation-warnings/+merge/141946
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?
--
https://code.launchpad.net/~hjd/widelands/compilation-warnings/+merge/141946
Your team Widelands Developers is requested to review the proposed merge of
lp:~hjd/widelands/compilation-warnings into lp:widelands.
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2012-11-30 18:16:47 +0000
+++ CMakeLists.txt 2013-01-04 15:56:24 +0000
@@ -157,8 +157,9 @@
set (PARAMETER_COMPILERFLAG_OLDSTYLECAST_EXTENDED "-Wold-style-cast -isystem ${Boost_INCLUDE_DIR}")
set (PARAMETER_COMPILERFLAG_OLDSTYLECAST "-Wold-style-cast")
-set (PARAMETER_COMPILERFLAG_GENERICWARNINGS "-Wno-attributes -Wall")
-set (PARAMETER_COMPILERFLAG_EXTRAWARNINGS "-Wextra -Wsign-promo")
+set (PARAMETER_COMPILERFLAG_GENERICWARNINGS "-Wall -Wextra")
+set (PARAMETER_COMPILERFLAG_EXTRAWARNINGS "-Wsign-promo")
+set (PARAMETER_COMPILERFLAG_GCCWARNINGS "-Wlogical-op")
set (PARAMETER_COMPILERFLAG_STRICT "-Werror -Wno-error=old-style-cast -Wno-error=deprecated-declarations -fdiagnostics-show-option")
IF (CMAKE_BUILD_TYPE STREQUAL "Debug")
include(CheckCXXCompilerFlag) #this include should be safe
@@ -198,12 +199,21 @@
IF (Compiler_generic_warnings_supported)
set (WL_COMPILERFLAG_GENERICWARNINGS " ${PARAMETER_COMPILERFLAG_GENERICWARNINGS}") #the space is on purpose!
ENDIF (Compiler_generic_warnings_supported)
- IF (WL_EXTRAWARNINGS)
- CHECK_CXX_COMPILER_FLAG(${PARAMETER_COMPILERFLAG_EXTRAWARNINGS} Compiler_extra_warnings_supported)
- IF (Compiler_extra_warnings_supported)
- set (WL_COMPILERFLAG_EXTRAWARNINGS " ${PARAMETER_COMPILERFLAG_EXTRAWARNINGS}") #the space is on purpose!
- ENDIF (Compiler_extra_warnings_supported)
- ENDIF (WL_EXTRAWARNINGS)
+
+ CHECK_CXX_COMPILER_FLAG(${PARAMETER_COMPILERFLAG_EXTRAWARNINGS} Compiler_extra_warnings_supported)
+ IF (Compiler_extra_warnings_supported)
+ set (WL_COMPILERFLAG_EXTRAWARNINGS " ${PARAMETER_COMPILERFLAG_EXTRAWARNINGS}") #the space is on purpose!
+ ELSE (Compiler_extra_warnings_supported)
+ message("Warning: couldn't set the following compiler options: ${PARAMETER_COMPILERFLAG_EXTRAWARNINGS}. Most likely these options are available in a newer release of your compiler.")
+ ENDIF (Compiler_extra_warnings_supported)
+
+ CHECK_CXX_COMPILER_FLAG(${PARAMETER_COMPILERFLAG_GCCWARNINGS} Compiler_gcc_warnings_supported)
+ IF (Compiler_gcc_warnings_supported)
+ set (WL_COMPILERFLAG_GCCWARNINGS " ${PARAMETER_COMPILERFLAG_GCCWARNINGS}") #the space is on purpose!
+ ELSE (Compiler_gcc_warnings_supported)
+ message("Warning: could not add additional GCC-specific warning options: ${PARAMETER_COMPILERFLAG_GCCWARNINGS}. Most likely you are using a different compiler, like Clang/LLVM.")
+ ENDIF (Compiler_gcc_warnings_supported)
+
IF (WL_STRICT)
CHECK_CXX_COMPILER_FLAG(${PARAMETER_COMPILERFLAG_STRICT} Compiler_strict_mode_supported)
@@ -215,7 +225,7 @@
ENDIF (CMAKE_BUILD_TYPE STREQUAL "Debug")
# CMAKE only defines "-g", but we need -DDEBUG also, and we need -DNOPARACHUTE (for SDL) in Debug
-set (CMAKE_CXX_FLAGS_DEBUG "-g -DDEBUG -DNOPARACHUTE${WL_COMPILERFLAG_OLDSTYLECAST}${WL_COMPILERFLAG_GENERICWARNINGS}${WL_COMPILERFLAG_EXTRAWARNINGS}${WL_COMPILERFLAG_STRICT}" CACHE STRING "Set by widelands CMakeLists.txt" FORCE)
+set (CMAKE_CXX_FLAGS_DEBUG "-g -DDEBUG -DNOPARACHUTE${WL_COMPILERFLAG_OLDSTYLECAST}${WL_COMPILERFLAG_GENERICWARNINGS}${WL_COMPILERFLAG_EXTRAWARNINGS}${WL_COMPILERFLAG_GCCWARNINGS}${WL_COMPILERFLAG_STRICT}" CACHE STRING "Set by widelands CMakeLists.txt" FORCE)
#This can be removed if no one uses gcc 4.5.1 or 4.5.2 any more
IF (CMAKE_COMPILER_IS_GNUCXX)
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
