widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/compiler_warnings_062017 into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1697192 in widelands: "Cleanup after move to boost.net"
  https://bugs.launchpad.net/widelands/+bug/1697192

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/compiler_warnings_062017/+merge/325717

Trying to get rid of all compiler warnings.

This isn't ready yet, but I need the Travis builds for the compilers that I don't have.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/compiler_warnings_062017 into lp:widelands.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2017-06-01 13:50:48 +0000
+++ CMakeLists.txt	2017-06-15 04:21:58 +0000
@@ -129,8 +129,11 @@
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-missing-noreturn")
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-implicit-fallthrough")
 
-  # TODO(sirver): weak-vtables should be enabled, but leads to lot of errors right now.
-  wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-weak-vtables")
+  # It is impossible to write code that both GCC and Clang will like,
+  # so we have to switch off the warning for one of them.
+  # http://clang-developers.42468.n3.nabble.com/Question-on-Wswitch-enum-td4025927.html
+  wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-switch-enum")
+
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-unreachable-code")
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Wno-documentation")
 

=== modified file 'src/economy/economy.cc'
--- src/economy/economy.cc	2017-02-12 09:10:57 +0000
+++ src/economy/economy.cc	2017-06-15 04:21:58 +0000
@@ -431,7 +431,9 @@
 	RequestList::iterator const it = std::find(requests_.begin(), requests_.end(), &req);
 
 	if (it == requests_.end()) {
+		GCC_DIAG_OFF("-Wformat");
 		log("WARNING: remove_request(%p) not in list\n", &req);
+		GCC_DIAG_ON("-Wformat");
 		return;
 	}
 

=== modified file 'src/economy/ware_instance.cc'
--- src/economy/ware_instance.cc	2017-04-23 12:11:19 +0000
+++ src/economy/ware_instance.cc	2017-06-15 04:21:58 +0000
@@ -187,7 +187,9 @@
 
 WareInstance::~WareInstance() {
 	if (supply_) {
+		GCC_DIAG_OFF("-Wformat");
 		molog("Ware %u still has supply %p\n", descr_index_, supply_);
+		GCC_DIAG_ON("-Wformat");
 		delete supply_;
 	}
 }

=== modified file 'src/logic/map_objects/bob.cc'
--- src/logic/map_objects/bob.cc	2017-04-23 12:11:19 +0000
+++ src/logic/map_objects/bob.cc	2017-06-15 04:21:58 +0000
@@ -901,7 +901,9 @@
 
 /// Give debug information.
 void Bob::log_general_info(const EditorGameBase& egbase) {
+	GCC_DIAG_OFF("-Wformat"); // GCC warnings can't handle polymorphy with void*
 	molog("Owner: %p\n", owner_);
+	GCC_DIAG_ON("-Wformat");
 	molog("Postition: (%i, %i)\n", position_.x, position_.y);
 	molog("ActID: %i\n", actid_);
 	molog("ActScheduled: %s\n", actscheduled_ ? "true" : "false");
@@ -926,7 +928,9 @@
 		molog("* ivar2: %i\n", stack_[i].ivar2);
 		molog("* ivar3: %i\n", stack_[i].ivar3);
 
+		GCC_DIAG_OFF("-Wformat");
 		molog("* object pointer: %p\n", stack_[i].objvar1.get(egbase));
+		GCC_DIAG_ON("-Wformat");
 		molog("* svar1: %s\n", stack_[i].svar1.c_str());
 
 		molog("* coords: (%i, %i)\n", stack_[i].coords.x, stack_[i].coords.y);
@@ -934,7 +938,9 @@
 		for (Direction dir = FIRST_DIRECTION; dir <= LAST_DIRECTION; ++dir) {
 			molog(" %d", stack_[i].diranims.get_animation(dir));
 		}
+		GCC_DIAG_OFF("-Wformat");
 		molog("\n* path: %p\n", stack_[i].path);
+		GCC_DIAG_ON("-Wformat");
 		if (stack_[i].path) {
 			const Path& path = *stack_[i].path;
 			Path::StepVector::size_type nr_steps = path.get_nsteps();
@@ -947,9 +953,10 @@
 				molog("*  (%i, %i)\n", coords.x, coords.y);
 			}
 		}
+		GCC_DIAG_OFF("-Wformat");
 		molog("* route: %p\n", stack_[i].route);
-
 		molog("* program: %p\n", stack_[i].route);
+		GCC_DIAG_ON("-Wformat");
 	}
 }
 

=== modified file 'src/logic/map_objects/immovable.cc'
--- src/logic/map_objects/immovable.cc	2017-04-23 12:11:19 +0000
+++ src/logic/map_objects/immovable.cc	2017-06-15 04:21:58 +0000
@@ -1329,10 +1329,14 @@
 void PlayerImmovable::log_general_info(const EditorGameBase& egbase) {
 	BaseImmovable::log_general_info(egbase);
 
+	GCC_DIAG_OFF("-Wformat");
 	molog("this: %p\n", this);
 	molog("owner_: %p\n", owner_);
+	GCC_DIAG_ON("-Wformat");
 	molog("player_number: %i\n", owner_->player_number());
+	GCC_DIAG_OFF("-Wformat");
 	molog("economy_: %p\n", economy_);
+	GCC_DIAG_ON("-Wformat");
 }
 
 constexpr uint8_t kCurrentPacketVersionPlayerImmovable = 1;

=== modified file 'src/logic/map_objects/tribes/building.cc'
--- src/logic/map_objects/tribes/building.cc	2017-05-23 07:00:23 +0000
+++ src/logic/map_objects/tribes/building.cc	2017-06-15 04:21:58 +0000
@@ -664,7 +664,9 @@
 	PlayerImmovable::log_general_info(egbase);
 
 	molog("position: (%i, %i)\n", position_.x, position_.y);
+	GCC_DIAG_OFF("-Wformat");
 	molog("flag: %p\n", flag_);
+	GCC_DIAG_ON("-Wformat");
 	molog("* position: (%i, %i)\n", flag_->get_position().x, flag_->get_position().y);
 
 	molog("anim: %s\n", descr().get_animation_name(anim_).c_str());
@@ -673,7 +675,9 @@
 	molog("leave_time: %i\n", leave_time_);
 
 	molog("leave_queue.size(): %lu\n", static_cast<long unsigned int>(leave_queue_.size()));
+	GCC_DIAG_OFF("-Wformat");
 	molog("leave_allow.get(): %p\n", leave_allow_.get(egbase));
+	GCC_DIAG_ON("-Wformat");
 }
 
 void Building::add_worker(Worker& worker) {

=== modified file 'src/logic/map_objects/tribes/worker.cc'
--- src/logic/map_objects/tribes/worker.cc	2017-05-03 07:24:06 +0000
+++ src/logic/map_objects/tribes/worker.cc	2017-06-15 04:21:58 +0000
@@ -977,24 +977,34 @@
 	Bob::log_general_info(egbase);
 
 	if (upcast(PlayerImmovable, loc, location_.get(egbase))) {
+		GCC_DIAG_OFF("-Wformat");
 		molog("* Owner: (%p)\n", &loc->owner());
+		GCC_DIAG_ON("-Wformat");
 		molog("** Owner (plrnr): %i\n", loc->owner().player_number());
+		GCC_DIAG_OFF("-Wformat");
 		molog("* Economy: %p\n", loc->get_economy());
+		GCC_DIAG_ON("-Wformat");
 	}
 
 	PlayerImmovable* imm = location_.get(egbase);
 	molog("location: %u\n", imm ? imm->serial() : 0);
+	GCC_DIAG_OFF("-Wformat");
 	molog("Economy: %p\n", economy_);
 	molog("transfer: %p\n", transfer_);
+	GCC_DIAG_ON("-Wformat");
 
 	if (upcast(WareInstance, ware, carried_ware_.get(egbase))) {
 		molog("* carried_ware->get_ware() (id): %i\n", ware->descr_index());
+		GCC_DIAG_OFF("-Wformat");
 		molog("* carried_ware->get_economy() (): %p\n", ware->get_economy());
+		GCC_DIAG_ON("-Wformat");
 	}
 
 	molog("current_exp: %i / %i\n", current_exp_, descr().get_needed_experience());
 
+	GCC_DIAG_OFF("-Wformat");
 	molog("supply: %p\n", supply_);
+	GCC_DIAG_ON("-Wformat");
 }
 
 /**

=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc	2017-05-14 06:50:28 +0000
+++ src/network/gamehost.cc	2017-06-15 04:21:58 +0000
@@ -65,7 +65,7 @@
 
 struct HostGameSettingsProvider : public GameSettingsProvider {
 	HostGameSettingsProvider(GameHost* const init_host)
-	   : host_(init_host), current_wincondition_(0) {
+	   : host_(init_host) {
 	}
 	~HostGameSettingsProvider() {
 	}
@@ -275,7 +275,6 @@
 
 private:
 	GameHost* host_;
-	int16_t current_wincondition_;
 	std::vector<std::string> wincondition_scripts_;
 };
 

=== modified file 'src/network/netclient.cc'
--- src/network/netclient.cc	2017-06-10 16:36:29 +0000
+++ src/network/netclient.cc	2017-06-15 04:21:58 +0000
@@ -68,8 +68,13 @@
 		return;
 
 	boost::system::error_code ec;
+#ifdef NDEBUG
+   boost::asio::write(socket_, boost::asio::buffer(packet.get_data(), packet.get_size()), ec);
+#else
 	size_t written =
 	   boost::asio::write(socket_, boost::asio::buffer(packet.get_data(), packet.get_size()), ec);
+#endif
+
 	// TODO(Notabilis): This one is an assertion of mine, I am not sure if it will hold
 	// If it doesn't, set the socket to blocking before writing
 	// If it does, remove this comment after build 20

=== modified file 'src/network/nethost.cc'
--- src/network/nethost.cc	2017-06-10 16:36:29 +0000
+++ src/network/nethost.cc	2017-06-15 04:21:58 +0000
@@ -170,8 +170,14 @@
 void NetHost::send(const ConnectionId id, const SendPacket& packet) {
 	boost::system::error_code ec;
 	if (is_connected(id)) {
+#ifdef NDEBUG
+		boost::asio::write(
+		   clients_.at(id).socket, boost::asio::buffer(packet.get_data(), packet.get_size()), ec);
+#else
 		size_t written = boost::asio::write(
 		   clients_.at(id).socket, boost::asio::buffer(packet.get_data(), packet.get_size()), ec);
+#endif
+
 		// TODO(Notabilis): This one is an assertion of mine, I am not sure if it will hold
 		// If it doesn't, set the socket to blocking before writing
 		// If it does, remove this comment after build 20

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2017-04-05 14:19:14 +0000
+++ src/scripting/lua_map.cc	2017-06-15 04:21:58 +0000
@@ -4458,7 +4458,7 @@
 		report_error(L, "<%s> is no valid warehouse policy!", str.c_str());
 }
 
-bool do_set_ware_policy(Warehouse* wh, const DescriptionIndex idx, const Warehouse::StockPolicy p) {
+inline bool do_set_ware_policy(Warehouse* wh, const DescriptionIndex idx, const Warehouse::StockPolicy p) {
 	wh->set_ware_policy(idx, p);
 	return true;
 }
@@ -4467,7 +4467,7 @@
  * Sets the given policy for the given ware in the given warehouse and return true.
  * If the no ware with the given name exists for the tribe of the warehouse, return false.
  */
-bool do_set_ware_policy(Warehouse* wh, const std::string& name, const Warehouse::StockPolicy p) {
+inline bool do_set_ware_policy(Warehouse* wh, const std::string& name, const Warehouse::StockPolicy p) {
 	const TribeDescr& tribe = wh->owner().tribe();
 	DescriptionIndex idx = tribe.ware_index(name);
 	if (!tribe.has_ware(idx)) {
@@ -4476,7 +4476,7 @@
 	return do_set_ware_policy(wh, idx, p);
 }
 
-bool do_set_worker_policy(Warehouse* wh,
+inline bool do_set_worker_policy(Warehouse* wh,
                           const DescriptionIndex idx,
                           const Warehouse::StockPolicy p) {
 	const TribeDescr& tribe = wh->owner().tribe();
@@ -4496,7 +4496,7 @@
  * policy.
  * If no worker with the given name exists for the tribe of the warehouse, return false.
  */
-bool do_set_worker_policy(Warehouse* wh, const std::string& name, const Warehouse::StockPolicy p) {
+inline bool do_set_worker_policy(Warehouse* wh, const std::string& name, const Warehouse::StockPolicy p) {
 	const TribeDescr& tribe = wh->owner().tribe();
 	DescriptionIndex idx = tribe.worker_index(name);
 	if (!tribe.has_worker(idx)) {
@@ -4566,11 +4566,11 @@
 
 // Gets the warehouse policy by ware/worker-name or id
 #define WH_GET_POLICY(type)                                                                        \
-	void do_get_##type##_policy(lua_State* L, Warehouse* wh, const DescriptionIndex idx) {          \
+	inline void do_get_##type##_policy(lua_State* L, Warehouse* wh, const DescriptionIndex idx) {          \
 		wh_policy_to_string(L, wh->get_##type##_policy(idx));                                        \
 	}                                                                                               \
                                                                                                    \
-	bool do_get_##type##_policy(lua_State* L, Warehouse* wh, const std::string& name) {             \
+	inline bool do_get_##type##_policy(lua_State* L, Warehouse* wh, const std::string& name) {             \
 		const TribeDescr& tribe = wh->owner().tribe();                                               \
 		DescriptionIndex idx = tribe.type##_index(name);                                             \
 		if (!tribe.has_##type(idx)) {                                                                \