Merge pull request #10209 from Pokechu22/assert-fmt

Assertion and panic alert improvements

Languages/update-source-strings.sh

@@ -10,16 +10,10 @@ SRCDIR=Source
 find $SRCDIR -name '*.cpp' -o -name '*.h' -o -name '*.c' | \
 	xgettext -s -p ./Languages/po -o dolphin-emu.pot --package-name="Dolphin Emulator" \
 	--keyword=_ \
-	--keyword=wxTRANSLATE \
-	--keyword=AskYesNoT \
 	--keyword=AskYesNoFmtT \
-	--keyword=CriticalAlertT \
 	--keyword=CriticalAlertFmtT \
-	--keyword=PanicAlertT \
 	--keyword=PanicAlertFmtT \
-	--keyword=PanicYesNoT \
 	--keyword=PanicYesNoFmtT \
-	--keyword=SuccessAlertT \
 	--keyword=SuccessAlertFmtT \
 	--keyword=GetStringT \
 	--keyword=_trans \

Source/Android/jni/MainAndroid.cpp

@@ -5,7 +5,6 @@
 #include <UICommon/GameFile.h>
 #include <android/log.h>
 #include <android/native_window_jni.h>
-#include <cinttypes>
 #include <cstdio>
 #include <cstdlib>
 #include <jni.h>

Source/Core/AudioCommon/WASAPIStream.cpp

@@ -17,6 +17,7 @@
 #include <thread>
 
 #include "Common/Assert.h"
+#include "Common/HRWrap.h"
 #include "Common/Logging/Log.h"
 #include "Common/StringUtil.h"
 #include "Common/Thread.h"
@@ -66,11 +67,11 @@ static bool HandleWinAPI(std::string_view message, HRESULT result)
       error = "Audio endpoint already in use!";
       break;
     default:
-      error = TStrToUTF8(_com_error(result).ErrorMessage()).c_str();
+      error = Common::GetHResultMessage(result);
       break;
     }
 
-    ERROR_LOG_FMT(AUDIO, "WASAPI: {}: {}", message, error);
+    ERROR_LOG_FMT(AUDIO, "WASAPI: {}: {} ({:08x})", message, error, result);
   }
 
   return SUCCEEDED(result);

Source/Core/Common/Assert.h

@@ -13,31 +13,33 @@
   {                                                                                                \
     if (!(_a_))                                                                                    \
     {                                                                                              \
-      if (!PanicYesNo(_fmt_, ##__VA_ARGS__))                                                       \
+      if (!PanicYesNoFmtAssert(_t_,                                                                \
+                               "An error occurred.\n\n" _fmt_ "\n\n"                               \
+                               "  Condition: {}\n  File: {}\n  Line: {}\n  Function: {}\n\n"       \
+                               "Ignore and continue?",                                             \
+                               ##__VA_ARGS__, #_a_, __FILE__, __LINE__, __func__))                 \
         Crash();                                                                                   \
     }                                                                                              \
   } while (0)
 
-#define DEBUG_ASSERT_MSG(_t_, _a_, _msg_, ...)                                                     \
+#define DEBUG_ASSERT_MSG(_t_, _a_, _fmt_, ...)                                                     \
   do                                                                                               \
   {                                                                                                \
     if constexpr (Common::Log::MAX_LOGLEVEL >= Common::Log::LogLevel::LDEBUG)                      \
-    {                                                                                              \
-      if (!(_a_))                                                                                  \
-      {                                                                                            \
-        ERROR_LOG(_t_, _msg_, ##__VA_ARGS__);                                                      \
-        if (!PanicYesNo(_msg_, ##__VA_ARGS__))                                                     \
-          Crash();                                                                                 \
-      }                                                                                            \
-    }                                                                                              \
+      ASSERT_MSG(_t_, _a_, _fmt_, ##__VA_ARGS__);                                                  \
   } while (0)
 
 #define ASSERT(_a_)                                                                                \
   do                                                                                               \
   {                                                                                                \
-    ASSERT_MSG(MASTER_LOG, _a_,                                                                    \
-               _trans("An error occurred.\n\n  Line: %d\n  File: %s\n\nIgnore and continue?"),     \
-               __LINE__, __FILE__);                                                                \
+    if (!(_a_))                                                                                    \
+    {                                                                                              \
+      if (!PanicYesNoFmt("An error occurred.\n\n"                                                  \
+                         "  Condition: {}\n  File: {}\n  Line: {}\n  Function: {}\n\n"             \
+                         "Ignore and continue?",                                                   \
+                         #_a_, __FILE__, __LINE__, __func__))                                      \
+        Crash();                                                                                   \
+    }                                                                                              \
   } while (0)
 
 #define DEBUG_ASSERT(_a_)                                                                          \

Source/Core/Common/CMakeLists.txt

@@ -164,7 +164,7 @@ elseif(WIN32)
     winmm.lib
   )
   if (_M_X86_64)
-	  target_link_libraries(common PRIVATE opengl32.lib)
+    target_link_libraries(common PRIVATE opengl32.lib)
   endif()
 elseif (ANDROID)
   target_link_libraries(common
@@ -286,6 +286,10 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
   target_link_libraries(common PUBLIC dl rt)
 endif()
 
+if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
+  target_sources(common PUBLIC HRWrap.h HRWrap.cpp)
+endif()
+
 if(USE_UPNP)
   target_link_libraries(common PRIVATE Miniupnpc::miniupnpc)
 endif()

Source/Core/Common/ChunkFile.h

@@ -25,6 +25,8 @@
 #include <utility>
 #include <vector>
 
+#include <fmt/format.h>
+
 #include "Common/Assert.h"
 #include "Common/CommonTypes.h"
 #include "Common/EnumMap.h"
@@ -332,8 +334,8 @@ class PointerWrap
 
     case MODE_VERIFY:
       DEBUG_ASSERT_MSG(COMMON, !memcmp(data, *ptr, size),
-                       "Savestate verification failure: buf %p != %p (size %u).\n", data, *ptr,
-                       size);
+                       "Savestate verification failure: buf {} != {} (size {}).\n", fmt::ptr(data),
+                       fmt::ptr(*ptr), size);
       break;
     }
 

Source/Core/Common/Common.h

@@ -15,7 +15,7 @@
 #define CHECK_HEAP_INTEGRITY()                                                                     \
   {                                                                                                \
     if (!_CrtCheckMemory())                                                                        \
-      PanicAlert("memory corruption detected. see log.");                                          \
+      PanicAlertFmt("memory corruption detected. see log.");                                       \
   }
 // If you want to see how much a pain in the ass singletons are, for example:
 // {614} normal block at 0x030C5310, 188 bytes long.

Source/Core/Common/HRWrap.cpp

@@ -0,0 +1,17 @@
+// Copyright 2021 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "HRWrap.h"
+
+#include <comdef.h>
+#include "Common/StringUtil.h"
+
+namespace Common
+{
+std::string GetHResultMessage(HRESULT hr)
+{
+  // See https://stackoverflow.com/a/7008111
+  _com_error err(hr);
+  return TStrToUTF8(err.ErrorMessage());
+}
+}  // namespace Common

Source/Core/Common/HRWrap.h

@@ -0,0 +1,33 @@
+// Copyright 2021 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include <fmt/format.h>
+#include <string>
+#include <winerror.h>
+
+namespace Common
+{
+std::string GetHResultMessage(HRESULT hr);
+
+// Wrapper for HRESULT to be used with fmt.  Note that we can't create a fmt::formatter directly
+// for HRESULT as HRESULT is simply a typedef on long and not a distinct type.
+struct HRWrap
+{
+  constexpr explicit HRWrap(HRESULT hr) : m_hr(hr) {}
+  const HRESULT m_hr;
+};
+}  // namespace Common
+
+template <>
+struct fmt::formatter<Common::HRWrap>
+{
+  constexpr auto parse(fmt::format_parse_context& ctx) { return ctx.begin(); }
+  template <typename FormatContext>
+  auto format(const Common::HRWrap& hr, FormatContext& ctx)
+  {
+    return fmt::format_to(ctx.out(), "{} ({:#010x})", Common::GetHResultMessage(hr.m_hr),
+                          static_cast<u32>(hr.m_hr));
+  }
+};

Source/Core/Common/Image.cpp

@@ -80,7 +80,7 @@ bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u
     byte_per_pixel = 4;
     break;
   default:
-    ASSERT_MSG(FRAMEDUMP, false, "Invalid format %d", static_cast<int>(format));
+    ASSERT_MSG(FRAMEDUMP, false, "Invalid format {}", format);
     return false;
   }
 

Source/Core/Common/MsgHandler.cpp

@@ -106,10 +106,14 @@ std::string GetStringT(const char* string)
   return s_str_translator(string);
 }
 
-static bool ShowMessageAlert(std::string_view text, bool yes_no, MsgType style)
+static bool ShowMessageAlert(std::string_view text, bool yes_no, Common::Log::LogType log_type,
+                             MsgType style, const char* file, int line)
 {
   const char* caption = GetCaption(style);
-  ERROR_LOG_FMT(MASTER_LOG, "{}: {}", caption, text);
+  // Directly call GenericLogFmt rather than using the normal log macros so that we can use the
+  // caller's line file and line number
+  Common::Log::GenericLogFmt<2>(Common::Log::LogLevel::LERROR, log_type, file, line,
+                                FMT_STRING("{}: {}"), caption, text);
 
   // Panic alerts.
   if (style == MsgType::Warning && s_abort_on_panic_alert)
@@ -127,27 +131,13 @@ static bool ShowMessageAlert(std::string_view text, bool yes_no, MsgType style)
   return true;
 }
 
-// This is the first stop for gui alerts where the log is updated and the
-// correct window is shown, but only for legacy printf-style messages
-bool MsgAlert(bool yes_no, MsgType style, const char* format, ...)
-{
-  char buffer[2048];
-
-  va_list args;
-  va_start(args, format);
-  CharArrayFromFormatV(buffer, sizeof(buffer) - 1, s_str_translator(format).c_str(), args);
-  va_end(args);
-
-  return ShowMessageAlert(buffer, yes_no, style);
-}
-
 // This is the first stop for gui alerts where the log is updated and the
 // correct window is shown, when using fmt
-bool MsgAlertFmtImpl(bool yes_no, MsgType style, fmt::string_view format,
-                     const fmt::format_args& args)
+bool MsgAlertFmtImpl(bool yes_no, MsgType style, Common::Log::LogType log_type, const char* file,
+                     int line, fmt::string_view format, const fmt::format_args& args)
 {
   const auto message = fmt::vformat(format, args);
 
-  return ShowMessageAlert(message, yes_no, style);
+  return ShowMessageAlert(message, yes_no, log_type, style, file, line);
 }
 }  // namespace Common