← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~hjd/widelands/more-warnings into lp:widelands

 

Hans Joachim Desserud has proposed merging lp:~hjd/widelands/more-warnings into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1033615 in widelands: "Consider checking for more warnings when compiling Widelands"
  https://bugs.launchpad.net/widelands/+bug/1033615

For more details, see:
https://code.launchpad.net/~hjd/widelands/more-warnings/+merge/148949

Adding another batch of additional warnings. Mainly finds a couple of issues reported by -Wswitch-enum and -Wswitch-default, as well as a couple of others which doesn't seem to hurt.

As mentioned, this includes -Wswitch-enum which has been silenced in several places, since some places will trigger warnings on 200+ SDL_* enums not used. I hope this won't ignore any important warnings, though adding 200 different elements just to have them there doesn't seem ideal either.

Some notes:
The two methods touched in src/ui_fsmenu/options.cc look identical to me, but I haven't investigated thoroughly.
Had to add explicit brackets to a conditional in editorinteractive.cc, otherwise it would fail to compile claiming it found an else without an if.

-- 
https://code.launchpad.net/~hjd/widelands/more-warnings/+merge/148949
Your team Widelands Developers is requested to review the proposed merge of lp:~hjd/widelands/more-warnings into lp:widelands.
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-01-27 14:13:03 +0000
+++ CMakeLists.txt	2013-02-17 18:04:25 +0000
@@ -158,7 +158,7 @@
 set (PARAMETER_COMPILERFLAG_OLDSTYLECAST_EXTENDED "-Wold-style-cast -isystem ${Boost_INCLUDE_DIR}")
 set (PARAMETER_COMPILERFLAG_OLDSTYLECAST "-Wold-style-cast")
 set (PARAMETER_COMPILERFLAG_GENERICWARNINGS "-Wall -Wextra")
-set (PARAMETER_COMPILERFLAG_EXTRAWARNINGS "-Wshadow -Wsign-promo -Wundef -Wunused -Wformat -Wformat-nonliteral -Wformat-security")
+set (PARAMETER_COMPILERFLAG_EXTRAWARNINGS "-Wformat -Wformat-nonliteral -Wformat-security -Wformat-y2k -Winit-self -Winvalid-pch -Wmissing-include-dirs -Woverlength-strings -Wpacked -Wpointer-arith -Wredundant-decls -Wshadow -Wsign-promo -Wswitch-default -Wswitch-enum -Wundef -Wunused -Wunused-macros")
 set (PARAMETER_COMPILERFLAG_GCCWARNINGS "-Wlogical-op -Wsync-nand -Wtrampolines")
 set (PARAMETER_COMPILERFLAG_STRICT "-Werror -Wno-error=old-style-cast -Wno-error=deprecated-declarations -fdiagnostics-show-option")
 IF (CMAKE_BUILD_TYPE STREQUAL "Debug")

=== modified file 'src/editor/editorinteractive.cc'
--- src/editor/editorinteractive.cc	2013-02-10 20:07:27 +0000
+++ src/editor/editorinteractive.cc	2013-02-17 18:04:25 +0000
@@ -333,8 +333,12 @@
 bool Editor_Interactive::handle_key(bool const down, SDL_keysym const code) {
 	bool handled = Interactive_Base::handle_key(down, code);
 
-	if (down)
+	if (down) {
 		// only on down events
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 			// Sel radius
 		case SDLK_1:
@@ -463,8 +467,15 @@
 			break;
 
 		}
-	else
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
+	} else {
 		// key up events
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_LSHIFT:
 		case SDLK_RSHIFT:
@@ -478,7 +489,10 @@
 		default:
 			break;
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
 
+	}
 	return handled;
 }
 

=== modified file 'src/minizip/unzip.cc'
--- src/minizip/unzip.cc	2013-01-06 11:02:56 +0000
+++ src/minizip/unzip.cc	2013-02-17 18:04:25 +0000
@@ -1,6 +1,9 @@
 /* Don't want to modify the minizip/zlib sources, so lets silence the warnings */
 #include "compile_diagnostics.h"
 GCC_DIAG_OFF("-Wold-style-cast")
+GCC_DIAG_OFF("-Wswitch-default")
+GCC_DIAG_OFF("-Wunused-macros")
+CLANG_DIAG_OFF("-Wunused-macros")
 
 /*
 ================================================================================

=== modified file 'src/scripting/pluto.cc'
--- src/scripting/pluto.cc	2013-02-10 17:39:24 +0000
+++ src/scripting/pluto.cc	2013-02-17 18:04:25 +0000
@@ -30,7 +30,9 @@
 // Widelands: silence warnings about unused variables, usually because they
 //are only used in conditional asserts
 #include "compile_diagnostics.h"
+GCC_DIAG_OFF("-Wunused-macros")
 GCC_DIAG_OFF("-Wunused-variable")
+CLANG_DIAG_OFF("-Wunused-macros")
 CLANG_DIAG_OFF("-Wunused-variable")
 
 // Forward declarated from lua_impl.h. So we do not need to include it

=== modified file 'src/ui_basic/editbox.cc'
--- src/ui_basic/editbox.cc	2013-02-10 19:36:24 +0000
+++ src/ui_basic/editbox.cc	2013-02-17 18:04:25 +0000
@@ -223,6 +223,10 @@
 bool EditBox::handle_key(bool const down, SDL_keysym const code)
 {
 	if (down) {
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_ESCAPE:
 			cancel();
@@ -392,6 +396,9 @@
 			}
 			break;
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return Panel::handle_key(down, code);

=== modified file 'src/ui_basic/listselect.cc'
--- src/ui_basic/listselect.cc	2013-02-10 19:36:24 +0000
+++ src/ui_basic/listselect.cc	2013-02-17 18:04:25 +0000
@@ -478,6 +478,10 @@
 bool BaseListselect::handle_key(bool const down, SDL_keysym const code) {
 	if (down) {
 		uint32_t selected_idx;
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_KP2:
 			if (code.mod & KMOD_NUM)
@@ -509,6 +513,9 @@
 		default:
 			break; // not handled
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return UI::Panel::handle_key(down, code);

=== modified file 'src/ui_basic/messagebox.cc'
--- src/ui_basic/messagebox.cc	2013-02-09 23:36:30 +0000
+++ src/ui_basic/messagebox.cc	2013-02-17 18:04:25 +0000
@@ -20,6 +20,7 @@
 #include "messagebox.h"
 
 #include "button.h"
+#include "compile_diagnostics.h"
 #include "constants.h"
 #include "graphic/font_handler.h"
 #include "graphic/graphic.h"
@@ -146,6 +147,9 @@
 	if (!down)
 		return false;
 
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 	switch (code.sym)
 	{
 	case SDLK_KP_ENTER:
@@ -160,6 +164,8 @@
 	default:
 		return false;
 	}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
 
 	return UI::Panel::handle_key(down, code);
 }

=== modified file 'src/ui_basic/multilineeditbox.cc'
--- src/ui_basic/multilineeditbox.cc	2013-02-10 19:36:24 +0000
+++ src/ui_basic/multilineeditbox.cc	2013-02-17 18:04:25 +0000
@@ -266,6 +266,10 @@
 bool Multiline_Editbox::handle_key(bool const down, SDL_keysym const code)
 {
 	if (down) {
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_KP_PERIOD:
 			if (code.mod & KMOD_NUM)
@@ -443,6 +447,9 @@
 			}
 			break;
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 		return true;
 	}
 

=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc	2013-02-10 19:36:24 +0000
+++ src/ui_basic/table.cc	2013-02-17 18:04:25 +0000
@@ -322,6 +322,10 @@
 bool Table<void *>::handle_key(bool down, SDL_keysym code)
 {
 	if (down) {
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_UP:
 		case SDLK_KP8:
@@ -336,6 +340,9 @@
 		default:
 			break; // not handled
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return UI::Panel::handle_key(down, code);

=== modified file 'src/ui_fsmenu/loadgame.cc'
--- src/ui_fsmenu/loadgame.cc	2013-02-10 19:36:24 +0000
+++ src/ui_fsmenu/loadgame.cc	2013-02-17 18:04:25 +0000
@@ -252,6 +252,9 @@
 	if (!down)
 		return false;
 
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 	switch (code.sym)
 	{
 	case SDLK_KP2:
@@ -283,6 +286,8 @@
 	default:
 		break;
 	}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
 
 	return Fullscreen_Menu_Base::handle_key(down, code);
 }

=== modified file 'src/ui_fsmenu/loadreplay.cc'
--- src/ui_fsmenu/loadreplay.cc	2013-02-10 19:36:24 +0000
+++ src/ui_fsmenu/loadreplay.cc	2013-02-17 18:04:25 +0000
@@ -222,6 +222,9 @@
 	if (!down)
 		return false;
 
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 	switch (code.sym)
 	{
 	case SDLK_KP2:
@@ -253,6 +256,8 @@
 	default:
 		break; // not handled
 	}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
 
 	return Fullscreen_Menu_Base::handle_key(down, code);
 }

=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc	2013-02-09 23:36:30 +0000
+++ src/ui_fsmenu/options.cc	2013-02-17 18:04:25 +0000
@@ -357,6 +357,10 @@
 {
 	if (down)
 	{
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym)
 		{
 			case SDLK_KP_ENTER:
@@ -369,6 +373,9 @@
 			default:
 				break; // not handled
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return Fullscreen_Menu_Base::handle_key(down, code);
@@ -595,6 +602,10 @@
 {
 	if (down)
 	{
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym)
 		{
 			case SDLK_KP_ENTER:
@@ -607,6 +618,9 @@
 			default:
 				break; // not handled
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return Fullscreen_Menu_Base::handle_key(down, code);

=== modified file 'src/wui/game_message_menu.cc'
--- src/wui/game_message_menu.cc	2013-02-10 19:36:24 +0000
+++ src/wui/game_message_menu.cc	2013-02-17 18:04:25 +0000
@@ -253,6 +253,10 @@
 bool GameMessageMenu::handle_key(bool down, SDL_keysym code)
 {
 	if (down) {
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_g:
 			if (m_centerviewbtn->enabled())
@@ -270,6 +274,9 @@
 		default:
 			break; // not handled
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return list->handle_key(down, code);

=== modified file 'src/wui/interactive_base.cc'
--- src/wui/interactive_base.cc	2013-02-10 20:07:27 +0000
+++ src/wui/interactive_base.cc	2013-02-17 18:04:25 +0000
@@ -841,6 +841,9 @@
 	if (m->quicknavigation->handle_key(down, code))
 		return true;
 
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 	switch (code.sym) {
 	case SDLK_KP9:
 		if (code.mod & KMOD_NUM)
@@ -889,6 +892,8 @@
 	default:
 		break;
 	}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
 
 	return Map_View::handle_key(down, code);
 }

=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc	2013-02-10 20:07:27 +0000
+++ src/wui/interactive_player.cc	2013-02-17 18:04:25 +0000
@@ -405,6 +405,10 @@
 bool Interactive_Player::handle_key(bool const down, SDL_keysym const code)
 {
 	if (down) {
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_SPACE:
 			toggle_buildhelp();
@@ -462,6 +466,9 @@
 		default:
 			break;
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
+
 	}
 
 	return Interactive_GameBase::handle_key(down, code);

=== modified file 'src/wui/interactive_spectator.cc'
--- src/wui/interactive_spectator.cc	2013-02-09 23:36:30 +0000
+++ src/wui/interactive_spectator.cc	2013-02-17 18:04:25 +0000
@@ -250,6 +250,10 @@
 bool Interactive_Spectator::handle_key(bool const down, SDL_keysym const code)
 {
 	if (down)
+
+//Will complain about 200+ SDL_ enums not checked if not silenced.
+GCC_DIAG_OFF("-Wswitch-enum")
+CLANG_DIAG_OFF("-Wswitch-enum")
 		switch (code.sym) {
 		case SDLK_m:
 			toggle_minimap();
@@ -287,6 +291,8 @@
 		default:
 			break;
 		}
+CLANG_DIAG_ON("-Wswitch-enum")
+GCC_DIAG_ON("-Wswitch-enum")
 
 	return Interactive_GameBase::handle_key(down, code);
 }