Debug: Use fmt for assert_msg/assert_msg_tile

src/CMakeLists.txt

@@ -126,6 +126,7 @@ add_files(
     date_type.h
     debug.cpp
     debug.h
+    debug_dbg_assert.h
     debug_desync.h
     debug_settings.h
     debug_tictoc.h

src/blitter/32bpp_base.cpp

@@ -8,6 +8,7 @@
 /** @file 32bpp_base.cpp Implementation of base for 32 bpp blitters. */
 
 #include "../stdafx.h"
+#include "../debug.h"
 #include "32bpp_base.hpp"
 #include "common.hpp"
 
@@ -144,7 +145,7 @@ void Blitter_32bppBase::ScrollBuffer(void *video, int left, int top, int width,
 		/* Decrease height and increase top */
 		top += scroll_y;
 		height -= scroll_y;
-		assert_msg(height > 0, "%d, %d, %d, %d, %d, %d", left, top, width, height, scroll_x, scroll_y);
+		assert_msg(height > 0, "{}, {}, {}, {}, {}, {}", left, top, width, height, scroll_x, scroll_y);
 
 		/* Adjust left & width */
 		if (scroll_x >= 0) {
@@ -168,7 +169,7 @@ void Blitter_32bppBase::ScrollBuffer(void *video, int left, int top, int width,
 
 		/* Decrease height. (scroll_y is <=0). */
 		height += scroll_y;
-		assert_msg(height > 0, "%d, %d, %d, %d, %d, %d", left, top, width, height, scroll_x, scroll_y);
+		assert_msg(height > 0, "{}, {}, {}, {}, {}, {}", left, top, width, height, scroll_x, scroll_y);
 
 		/* Adjust left & width */
 		if (scroll_x >= 0) {

src/cachecheck.cpp

@@ -529,12 +529,12 @@ void CheckCaches(bool force_check, std::function<void(std::string_view)> log, Ch
 		ValidateVehicleTickCaches();
 
 		for (Vehicle *v : Vehicle::Iterate()) {
-			if (v->Previous()) assert_msg(v->Previous()->Next() == v, "%u", v->index);
-			if (v->Next()) assert_msg(v->Next()->Previous() == v, "%u", v->index);
+			if (v->Previous()) assert_msg(v->Previous()->Next() == v, "{}", v->index);
+			if (v->Next()) assert_msg(v->Next()->Previous() == v, "{}", v->index);
 		}
 		for (const TemplateVehicle *tv : TemplateVehicle::Iterate()) {
-			if (tv->Prev()) assert_msg(tv->Prev()->Next() == tv, "%u", tv->index);
-			if (tv->Next()) assert_msg(tv->Next()->Prev() == tv, "%u", tv->index);
+			if (tv->Prev()) assert_msg(tv->Prev()->Next() == tv, "{}", tv->index);
+			if (tv->Next()) assert_msg(tv->Next()->Prev() == tv, "{}", tv->index);
 		}
 
 		{

src/cargopacket.cpp

@@ -8,6 +8,7 @@
 /** @file cargopacket.cpp Implementation of the cargo packets. */
 
 #include "stdafx.h"
+#include "debug.h"
 #include "station_base.h"
 #include "core/pool_func.hpp"
 #include "core/random_func.hpp"
@@ -524,6 +525,22 @@ void VehicleCargoList::PopCargo(Taction action)
 	}
 }
 
+void VehicleCargoList::AssertCountConsistencyError() const
+{
+	assert_msg(this->action_counts[MTA_KEEP] +
+			this->action_counts[MTA_DELIVER] +
+			this->action_counts[MTA_TRANSFER] +
+			this->action_counts[MTA_LOAD] == this->count,
+			"{} + {} + {} + {} != {}, ({} in {} packets)",
+			this->action_counts[MTA_KEEP],
+			this->action_counts[MTA_DELIVER],
+			this->action_counts[MTA_TRANSFER],
+			this->action_counts[MTA_LOAD],
+			this->count,
+			this->RecalculateCargoTotal(),
+			this->packets.size());
+}
+
 /**
  * Update the cached values to reflect the removal of this packet or part of it.
  * Decreases count, feeder share and periods_in_transit.

src/cargopacket.h

@@ -403,25 +403,22 @@ class VehicleCargoList : public CargoList<VehicleCargoList, CargoPacketList> {
 		return total;
 	}
 
+	void AssertCountConsistencyError() const;
 public:
 
 	/**
 	 * Assert that the designation counts add up.
 	 */
 	inline void AssertCountConsistency() const
 	{
-		assert_msg(this->action_counts[MTA_KEEP] +
+#ifdef WITH_ASSERT
+		if (unlikely(this->action_counts[MTA_KEEP] +
 				this->action_counts[MTA_DELIVER] +
 				this->action_counts[MTA_TRANSFER] +
-				this->action_counts[MTA_LOAD] == this->count,
-				"%u + %u + %u + %u != %u, (%u in %u packets)",
-				this->action_counts[MTA_KEEP],
-				this->action_counts[MTA_DELIVER],
-				this->action_counts[MTA_TRANSFER],
-				this->action_counts[MTA_LOAD],
-				this->count,
-				this->RecalculateCargoTotal(),
-				(uint) this->packets.size());
+				this->action_counts[MTA_LOAD] != this->count)) {
+			this->AssertCountConsistencyError();
+		}
+#endif
 	}
 
 protected:

src/command.cpp

@@ -1268,8 +1268,8 @@ CommandCost DoCommandPInternal(TileIndex tile, uint32_t p1, uint32_t p2, uint64_
 	 * i.e. cost and error state are the same. */
 	if (!test_and_exec_can_differ) {
 		assert_msg(res.GetCost() == res2.GetCost() && res.Failed() == res2.Failed(),
-				"Command: cmd: 0x%X (%s), Test: %s, Exec: %s", cmd, GetCommandName(cmd),
-				res.SummaryMessage(GB(cmd, 16, 16)).c_str(), res2.SummaryMessage(GB(cmd, 16, 16)).c_str()); // sanity check
+				"Command: cmd: 0x{:X} ({}), Test: {}, Exec: {}", cmd, GetCommandName(cmd),
+				res.SummaryMessage(GB(cmd, 16, 16)), res2.SummaryMessage(GB(cmd, 16, 16))); // sanity check
 	} else if (res2.Failed()) {
 		return_dcpi(res2);
 	}

src/core/pool_type.hpp

@@ -11,6 +11,7 @@
 #define POOL_TYPE_HPP
 
 #include "enum_type.hpp"
+#include "debug_dbg_assert.h"
 #include <vector>
 
 /** Various types of a pool. */
@@ -119,7 +120,7 @@ struct Pool : PoolBase {
 
 	inline PtrType &GetRawRef(size_t index)
 	{
-		dbg_assert_msg(index < this->first_unused, "index: " PRINTF_SIZE ", first_unused: " PRINTF_SIZE ", name: %s", index, this->first_unused, this->name);
+		dbg_assert_msg(index < this->first_unused, "index: {}, first_unused: {}, name: {}", index, this->first_unused, this->name);
 		return this->data[index];
 	}
 
@@ -296,7 +297,7 @@ struct Pool : PoolBase {
 		{
 			if (p == nullptr) return;
 			Titem *pn = static_cast<Titem *>(p);
-			dbg_assert_msg(pn == Tpool->Get(pn->index), "name: %s", Tpool->name);
+			dbg_assert_msg(pn == Tpool->Get(pn->index), "name: {}", Tpool->name);
 			Tpool->FreeItem(pn->index);
 		}
 
@@ -329,7 +330,7 @@ struct Pool : PoolBase {
 				 * memory are the same (because of possible inheritance).
 				 * Use { size_t index = item->index; delete item; new (index) item; }
 				 * instead to make sure destructor is called and no memory leaks. */
-				dbg_assert_msg(ptr != Tpool->data[i], "name: %s", Tpool->name);
+				dbg_assert_msg(ptr != Tpool->data[i], "name: {}", Tpool->name);
 			}
 			return ptr;
 		}

src/debug.cpp

@@ -18,6 +18,7 @@
 #include "settings_type.h"
 #include "date_func.h"
 #include "thread.h"
+#include "map_func.h"
 #include <array>
 #include <mutex>
 
@@ -449,3 +450,29 @@ void TicToc::PrintAndReset()
 	this->state.count = 0;
 	this->state.chrono_sum = 0;
 }
+
+[[noreturn]] void AssertMsgErrorVFmt(int line, const char *file, const char *expr, fmt::string_view msg, fmt::format_args args)
+{
+	format_buffer out;
+	out.vformat(msg, args);
+
+	assert_str_error(line, file, expr, out);
+}
+
+[[noreturn]] void AssertMsgTileErrorVFmt(int line, const char *file, const char *expr, uint32_t tile, fmt::string_view msg, fmt::format_args args)
+{
+	format_buffer out;
+	DumpTileInfo(out, tile);
+	out.append(", ");
+	out.vformat(msg, args);
+
+	assert_str_error(line, file, expr, out);
+}
+
+void assert_tile_error(int line, const char *file, const char *expr, uint32_t tile)
+{
+	format_buffer out;
+	DumpTileInfo(out, tile);
+
+	assert_str_error(line, file, expr, out);
+}

src/debug.h

@@ -116,4 +116,25 @@ void DumpDesyncMsgLog(struct format_target &buffer);
 void DebugSendRemoteMessages();
 void DebugReconsiderSendRemoteMessages();
 
+template <typename... T>
+[[noreturn]] void AssertMsgError(int line, const char *file, const char *expr, fmt::format_string<T...> msg, T&&... args)
+{
+	[[noreturn]] extern void AssertMsgErrorVFmt(int line, const char *file, const char *expr, fmt::string_view msg, fmt::format_args args);
+	AssertMsgErrorVFmt(line, file, expr, msg, fmt::make_format_args(args...));
+}
+template <typename... T>
+[[noreturn]] void AssertMsgTileError(int line, const char *file, const char *expr, uint32_t tile, fmt::format_string<T...> msg, T&&... args)
+{
+	[[noreturn]] extern void AssertMsgTileErrorVFmt(int line, const char *file, const char *expr, uint32_t tile, fmt::string_view msg, fmt::format_args args);
+	AssertMsgTileErrorVFmt(line, file, expr, tile, msg, fmt::make_format_args(args...));
+}
+
+#if !defined(NDEBUG) || defined(WITH_ASSERT)
+#	define assert_msg(expression, ...) do { if (unlikely(!(expression))) AssertMsgError(__LINE__, __FILE__, #expression, __VA_ARGS__); } while (false)
+#	define assert_msg_tile(expression, tile, ...) do { if (unlikely(!(expression))) AssertMsgTileError(__LINE__, __FILE__, #expression, tile, __VA_ARGS__); } while (false)
+#else
+#	define assert_msg(expression, ...)
+#	define assert_msg_tile(expression, tile, ...)
+#endif
+
 #endif /* DEBUG_H */

src/debug_dbg_assert.h

@@ -0,0 +1,22 @@
+/*
+ * This file is part of OpenTTD.
+ * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2.
+ * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/** @file debug_dbg_assert.h Macros for normally-off extended debug asserts. */
+
+#ifndef DEBUG_DBG_ASSERT_H
+#define DEBUG_DBG_ASSERT_H
+
+#ifdef WITH_FULL_ASSERTS
+#	include "debug.h"
+#	define dbg_assert_msg(expression, ...) assert_msg(expression, __VA_ARGS__)
+#	define dbg_assert_msg_tile(expression, tile, ...) assert_msg_tile(expression, tile, __VA_ARGS__)
+#else
+#	define dbg_assert_msg(expression, ...)
+#	define dbg_assert_msg_tile(expression, tile, ...)
+#endif
+
+#endif /* DEBUG_DBG_ASSERT_H */

src/network/core/udp.cpp

@@ -103,7 +103,7 @@ void NetworkUDPSocketHandler::SendPacket(Packet &p, NetworkAddress &recv, bool a
 			this->SendPacket(frag, recv, all, broadcast, short_mtu);
 			frag.ResetState(PACKET_UDP_EX_MULTI);
 		}
-		assert_msg(current_frag == frag_count, "%u, %u", current_frag, frag_count);
+		assert_msg(current_frag == frag_count, "{}, {}", current_frag, frag_count);
 		return;
 	}
 

src/newgrf.cpp

@@ -11872,7 +11872,7 @@ void LoadNewGRF(uint load_index, uint num_baseset)
 				SetBit(c->flags, GCF_RESERVED);
 			} else if (stage == GLS_ACTIVATION) {
 				ClrBit(c->flags, GCF_RESERVED);
-				assert_msg(GetFileByGRFID(c->ident.grfid) == _cur.grffile, "%08X", BSWAP32(c->ident.grfid));
+				assert_msg(GetFileByGRFID(c->ident.grfid) == _cur.grffile, "{:08X}", BSWAP32(c->ident.grfid));
 				ClearTemporaryNewGRFData(_cur.grffile);
 				BuildCargoTranslationMap();
 				HandleVarAction2OptimisationPasses();

src/newgrf_text.cpp

@@ -695,11 +695,6 @@ const char *GetDefaultLangGRFStringFromGRFText(const GRFTextWrapper &text)
  */
 const char *GetGRFStringPtr(uint32_t stringid)
 {
-#if 0
-	assert_msg(stringid < _grf_text.size(), "stringid: %u, size: %u", stringid, (uint)_grf_text.size());
-	assert_msg(_grf_text[stringid].grfid != 0, "stringid: %u", stringid);
-#endif
-
 	if (stringid >= _grf_text.size() || _grf_text[stringid].grfid == 0) {
 		Debug(misc, 0, "Invalid NewGRF string ID: {}", stringid);
 		return "(invalid StringID)";

src/openttd.cpp

@@ -186,44 +186,6 @@ void FatalErrorI(const std::string &str)
 	fatalerror_common(str.c_str());
 }
 
-void CDECL assert_msg_error(int line, const char *file, const char *expr, const char *str, ...)
-{
-	if (CrashLog::HaveAlreadyCrashed()) DoOSAbort();
-
-	char buf[2048];
-
-	va_list va;
-	va_start(va, str);
-	vseprintf(buf, lastof(buf), str, va);
-	va_end(va);
-
-	assert_str_error(line, file, expr, buf);
-}
-
-void CDECL assert_msg_tile_error(int line, const char *file, const char *expr, uint32_t tile, const char *str, ...)
-{
-	char buf[2048];
-
-	format_to_fixed_z out(buf, lastof(buf));
-	DumpTileInfo(out, tile);
-	out.append(", ");
-
-	va_list va;
-	va_start(va, str);
-	vseprintf(out.finalise(), lastof(buf), str, va);
-	va_end(va);
-
-	assert_str_error(line, file, expr, buf);
-}
-
-void assert_tile_error(int line, const char *file, const char *expr, uint32_t tile)
-{
-	format_buffer out;
-	DumpTileInfo(out, tile);
-
-	assert_str_error(line, file, expr, out);
-}
-
 /**
  * Show the help message when someone passed a wrong parameter.
  */

src/order_cmd.cpp

@@ -888,16 +888,16 @@ void OrderList::DebugCheckSanity() const
 			check_total_duration += o->GetWaitTime() + o->GetTravelTime();
 		}
 	}
-	assert_msg(this->GetNumOrders() == check_num_orders, "%u, %u", (uint) this->GetNumOrders(), check_num_orders);
-	assert_msg(this->num_manual_orders == check_num_manual_orders, "%u, %u", this->num_manual_orders, check_num_manual_orders);
-	assert_msg(this->timetable_duration == check_timetable_duration, "%u, %u", this->timetable_duration, check_timetable_duration);
-	assert_msg(this->total_duration == check_total_duration, "%u, %u", this->total_duration, check_total_duration);
+	assert_msg(this->GetNumOrders() == check_num_orders, "{}, {}", this->GetNumOrders(), check_num_orders);
+	assert_msg(this->num_manual_orders == check_num_manual_orders, "{}, {}", this->num_manual_orders, check_num_manual_orders);
+	assert_msg(this->timetable_duration == check_timetable_duration, "{}, {}", this->timetable_duration, check_timetable_duration);
+	assert_msg(this->total_duration == check_total_duration, "{}, {}", this->total_duration, check_total_duration);
 
 	for (const Vehicle *v = this->first_shared; v != nullptr; v = v->NextShared()) {
 		++check_num_vehicles;
-		assert_msg(v->orders == this, "%p, %p", v->orders, this);
+		assert_msg(v->orders == this, "{}, {}", fmt::ptr(v->orders), fmt::ptr(this));
 	}
-	assert_msg(this->num_vehicles == check_num_vehicles, "%u, %u", this->num_vehicles, check_num_vehicles);
+	assert_msg(this->num_vehicles == check_num_vehicles, "{}, {}", this->num_vehicles, check_num_vehicles);
 	Debug(misc, 6, "... detected {} orders ({} manual), {} vehicles, {} timetabled, {} total",
 			this->GetNumOrders(), this->num_manual_orders,
 			this->num_vehicles, this->timetable_duration, this->total_duration);

src/pbs.cpp

@@ -107,7 +107,7 @@ bool TryReserveRailTrackdir(const Train *v, TileIndex tile, Trackdir td, bool tr
 bool TryReserveRailTrack(TileIndex tile, Track track, bool trigger_stations)
 {
 	assert_msg_tile((TrackdirBitsToTrackBits(GetTileTrackdirBits(tile, TRANSPORT_RAIL, 0)) & TrackToTrackBits(track)) != 0, tile,
-			"%X, %X, %X", TrackdirBitsToTrackBits(GetTileTrackdirBits(tile, TRANSPORT_RAIL, 0)), track, TrackToTrackBits(track));
+			"{:X}, {:X}, {:X}", TrackdirBitsToTrackBits(GetTileTrackdirBits(tile, TRANSPORT_RAIL, 0)), track, TrackToTrackBits(track));
 
 	if (_settings_client.gui.show_track_reservation) {
 		/* show the reserved rail if needed */
@@ -203,7 +203,7 @@ void UnreserveRailTrackdir(TileIndex tile, Trackdir td)
  */
 void UnreserveRailTrack(TileIndex tile, Track t)
 {
-	assert_msg_tile(TrackdirBitsToTrackBits(GetTileTrackdirBits(tile, TRANSPORT_RAIL, 0)) & TrackToTrackBits(t), tile, "track: %u", t);
+	assert_msg_tile(TrackdirBitsToTrackBits(GetTileTrackdirBits(tile, TRANSPORT_RAIL, 0)) & TrackToTrackBits(t), tile, "track: {:X}", t);
 
 	if (_settings_client.gui.show_track_reservation) {
 		if (IsTileType(tile, MP_TUNNELBRIDGE)) {
@@ -1310,7 +1310,7 @@ void FillTrainReservationLookAhead(Train *v)
  */
 Train *GetTrainForReservation(TileIndex tile, Track track)
 {
-	assert_msg_tile(HasReservedTracks(tile, TrackToTrackBits(track)), tile, "track: %u", track);
+	assert_msg_tile(HasReservedTracks(tile, TrackToTrackBits(track)), tile, "track: {:X}", track);
 	Trackdir  trackdir = TrackToTrackdir(track);
 
 	RailTypes rts = GetRailTypeInfo(GetTileRailTypeByTrack(tile, track))->all_compatible_railtypes;

src/rail.h

@@ -21,6 +21,7 @@
 #include "signal_type.h"
 #include "rail_map.h"
 #include "settings_type.h"
+#include "debug_dbg_assert.h"
 #include <vector>
 
 /** Railtype flag bit numbers. */
@@ -331,7 +332,7 @@ class RailTypeInfo {
 inline const RailTypeInfo *GetRailTypeInfo(RailType railtype)
 {
 	extern RailTypeInfo _railtypes[RAILTYPE_END];
-	dbg_assert_msg(railtype < RAILTYPE_END, "%u", railtype);
+	dbg_assert_msg(railtype < RAILTYPE_END, "{}", railtype);
 	return &_railtypes[railtype];
 }