CheckAndAddWarning: Enforce explicitely marked warnings as errors

I replaced the previous usage of `check_and_add_flag` with a new `check_and_add_warning` function. Certain warnings are also now errors by default. Those are mainly warnings that show and sneak up commonly but are quick to fix.

CMake/CheckAndAddWarning.cmake

@@ -0,0 +1,40 @@
+include(CheckAndAddFlag)
+
+# check_and_add_warning(<basename> <warning_name> [ERROR] [DEBUG_ONLY | RELEASE_ONLY])
+#
+# Add a compiler warning flag to the current scope.
+#
+# Can optionally add the flag to Debug or Release configurations only, use this when
+# targeting multi-configuration generators like Visual Studio or Xcode.
+# Release configurations means NOT Debug, so it will work for RelWithDebInfo or MinSizeRel too.
+#
+# If the flag is added successfully, the variables FLAG_C_<variable>_WARN/FLAG_CXX_<variable>_WARN and
+# FLAG_C_<variable>_ERROR/FLAG_CXX_<variable>_ERROR may be set to ON.
+#
+# Examples:
+#   check_and_add_warning(PEDANTIC pedantic)
+#   check_and_add_warning(CONV conversion ERROR RELEASE_ONLY)
+#   check_and_add_warning(SIGNCONV sign-conversion DEBUG_ONLY)
+#   check_and_add_warning(SHADOW shadow)
+
+function(check_and_add_warning basename warning_name)
+  set(is_error OFF)
+  
+  if(ARGV2 STREQUAL "ERROR")
+    set(is_error ON)
+  elseif(ARGV2)
+    message(FATAL_ERROR "check_and_add_warning called with incorrect arguments: ${ARGN}")
+  endif()
+  unset(ARGV2)
+
+  set(compilation_profile ${ARGV3})
+
+  set(warn_var "${basename}_WARN")
+  set(err_var  "${basename}_ERROR")
+
+  check_and_add_flag(${warn_var} "-W${warning_name}" ${compilation_profile})
+
+  if(is_error)
+    check_and_add_flag(${err_var} "-Werror=${warning_name}" ${compilation_profile})
+  endif()
+endfunction()

CMakeLists.txt

@@ -160,6 +160,7 @@ list(APPEND CMAKE_MODULE_PATH
 
 # Support functions
 include(CheckAndAddFlag)
+include(CheckAndAddWarning)
 include(CheckCCompilerFlag)
 include(DolphinCompileDefinitions)
 include(DolphinDisableWarningsMSVC)

Source/CMakeLists.txt

@@ -27,24 +27,25 @@ if (MSVC)
   # Compile PCH
   add_subdirectory(PCH)
 else()
-  check_and_add_flag(HAVE_WALL -Wall)
+  check_and_add_warning(HAVE_WALL all)
   # TODO: would like these but they produce overwhelming amounts of warnings
-  #check_and_add_flag(EXTRA -Wextra)
-  #check_and_add_flag(MISSING_FIELD_INITIALIZERS -Wmissing-field-initializers)
-  #check_and_add_flag(SWITCH_DEFAULT -Wswitch-default)
-  #check_and_add_flag(FLOAT_EQUAL -Wfloat-equal)
-  #check_and_add_flag(CONVERSION -Wconversion)
-  #check_and_add_flag(ZERO_AS_NULL_POINTER_CONSTANT -Wzero-as-null-pointer-constant)
-  check_and_add_flag(TYPE_LIMITS -Wtype-limits)
-  check_and_add_flag(SIGN_COMPARE -Wsign-compare)
-  check_and_add_flag(IGNORED_QUALIFIERS -Wignored-qualifiers)
-  check_and_add_flag(UNINITIALIZED -Wuninitialized)
-  check_and_add_flag(LOGICAL_OP -Wlogical-op)
-  check_and_add_flag(SHADOW -Wshadow)
-  check_and_add_flag(SHADOW_FIELD_IN_CONSTRUCTOR -Wshadow-field-in-constructor)
-  check_and_add_flag(INIT_SELF -Winit-self)
-  check_and_add_flag(MISSING_DECLARATIONS -Wmissing-declarations)
-  check_and_add_flag(MISSING_VARIABLE_DECLARATIONS -Wmissing-variable-declarations)
+  #check_and_add_warning(EXTRA extra)
+  #check_and_add_warning(MISSING_FIELD_INITIALIZERS missing-field-initializers)
+  #check_and_add_warning(SWITCH_DEFAULT switch-default)
+  #check_and_add_warning(FLOAT_EQUAL float-equal)
+  #check_and_add_warning(CONVERSION conversion)
+  #check_and_add_warning(ZERO_AS_NULL_POINTER_CONSTANT zero-as-null-pointer-constant)
+  check_and_add_warning(TYPE_LIMITS type-limits ERROR)
+  check_and_add_warning(SIGN_COMPARE sign-compare ERROR)
+  check_and_add_warning(IGNORED_QUALIFIERS ignored-qualifiers ERROR)
+  check_and_add_warning(UNINITIALIZED uninitialized ERROR)
+  check_and_add_warning(LOGICAL_OP logical-op ERROR)
+  check_and_add_warning(SHADOW shadow ERROR)
+  check_and_add_warning(SHADOW_FIELD_IN_CONSTRUCTOR shadow-field-in-constructor ERROR)
+  check_and_add_warning(INIT_SELF init-self ERROR)
+  check_and_add_warning(MISSING_DECLARATIONS missing-declarations ERROR)
+  check_and_add_warning(MISSING_VARIABLE_DECLARATIONS missing-variable-declarations ERROR)
+  check_and_add_warning(FORMAT format ERROR)
 
   # Disable -Wstringop-truncation warnings as they result in many false positives.
   # In most (all?) cases where std::strncpy is used, we want to fill the entire buffer
@@ -54,12 +55,6 @@ else()
   if (HAS_STRINGOP_TRUNCATION_WARNING)
     check_and_add_flag(NO_STRINGOP_TRUNCATION -Wno-stringop-truncation)
   endif()
-
-  # Format string issues that the compiler can detect should be compile time errors.
-  check_cxx_compiler_flag(-Wformat HAS_FORMAT_WARNING)
-  if (HAS_FORMAT_WARNING)
-    check_and_add_flag(FORMAT_WARNING_TO_ERROR -Werror=format)
-  endif()
 endif()
 
 # These aren't actually needed for C11/C++11

Source/Core/Core/HW/WiimoteEmu/Speaker.cpp

@@ -67,7 +67,7 @@ void SpeakerLogic::SpeakerData(const u8* data, int length, float speaker_pan)
 
   // Potentially 40 resulting samples.
   std::array<s16, WiimoteCommon::OutputReportSpeakerData::DATA_SIZE * 2> samples;
-  assert(length * 2 <= samples.size());
+  assert(length * 2 <= static_cast<int>(samples.size()));
 
   unsigned int sample_rate_dividend, sample_length;
   u8 volume_divisor;

Source/Core/Core/IOS/Network/IP/Top.cpp

@@ -256,7 +256,7 @@ static std::vector<InterfaceRouting> GetSystemInterfaceRouting()
   }
 
   // read response
-  int msg_len = 0;
+  unsigned int msg_len = 0;
   msg_buffer.fill(0);
 
   do
@@ -270,9 +270,10 @@ static std::vector<InterfaceRouting> GetSystemInterfaceRouting()
     }
 
     nl_msg = reinterpret_cast<nlmsghdr*>(buf_ptr);
-    if (NLMSG_OK(nl_msg, read_len) == 0)
+    if (NLMSG_OK(nl_msg, static_cast<unsigned int>(read_len)) == 0)
     {
-      ERROR_LOG_FMT(IOS_NET, "Received netlink error response ({})", NLMSG_OK(nl_msg, read_len));
+      ERROR_LOG_FMT(IOS_NET, "Received netlink error response ({})",
+                    NLMSG_OK(nl_msg, static_cast<unsigned int>(read_len)));
       return {};
     }
 

Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp

@@ -176,7 +176,10 @@ Arm64GPRCache::GuestRegInfo Arm64GPRCache::GetGuestCR(size_t preg)
 
 Arm64GPRCache::GuestRegInfo Arm64GPRCache::GetGuestByIndex(size_t index)
 {
-  if (index >= GUEST_GPR_OFFSET && index < GUEST_GPR_OFFSET + GUEST_GPR_COUNT)
+  // We do not need to test for `index >= GUEST_GPR_OFFSET` because
+  // GUEST_GPR_OFFSET is always 0. This otherwise raises a warning.
+  static_assert(GUEST_GPR_OFFSET == 0);
+  if (index < GUEST_GPR_OFFSET + GUEST_GPR_COUNT)
     return GetGuestGPR(index - GUEST_GPR_OFFSET);
   if (index >= GUEST_CR_OFFSET && index < GUEST_CR_OFFSET + GUEST_CR_COUNT)
     return GetGuestCR(index - GUEST_CR_OFFSET);

Source/Core/DolphinQt/Config/Mapping/MappingCommon.cpp

@@ -26,7 +26,7 @@ constexpr auto INPUT_DETECT_MAXIMUM_TIME = std::chrono::seconds(5);
 // Ignore the mouse-click when queuing more buttons with "alternate mappings" enabled.
 constexpr auto INPUT_DETECT_ENDING_IGNORE_TIME = std::chrono::milliseconds(50);
 
-bool ContainsAnalogInput(const ciface::Core::InputDetector::Results& results)
+static bool ContainsAnalogInput(const ciface::Core::InputDetector::Results& results)
 {
   return std::ranges::any_of(results, [](auto& detection) { return detection.smoothness > 1; });
 }

Source/Core/VideoBackends/Metal/MRCHelpers.h

@@ -20,7 +20,7 @@ template <typename T>
 class MRCOwned
 {
   T ptr;
-  MRCOwned(T ptr) : ptr(ptr) {}
+  MRCOwned(T raw_ptr) : ptr(raw_ptr) {}
 
 public:
   MRCOwned() : ptr(nullptr) {}

Source/Core/VideoBackends/Metal/MTLStateTracker.mm

@@ -32,7 +32,7 @@ struct Metal::StateTracker::Backref
 {
   std::mutex mtx;
   StateTracker* state_tracker;
-  explicit Backref(StateTracker* state_tracker) : state_tracker(state_tracker) {}
+  explicit Backref(StateTracker* tracker) : state_tracker(tracker) {}
 };
 
 struct Metal::StateTracker::PerfQueryTracker

Source/UnitTests/Common/SPSCQueueTest.cpp

@@ -54,10 +54,10 @@ TEST(SPSCQueue, MultiThreaded)
   auto popper = [&q]() {
     for (u32 i = 0; i < 100000; ++i)
     {
-      while (q.Empty())
-        ;
       u32 v;
-      q.Pop(v);
+      while (!q.Pop(v))
+      {
+      }
       EXPECT_EQ(i, v);
     }
   };

Source/UnitTests/Common/StringUtilTest.cpp

@@ -57,7 +57,7 @@ static void DoRoundTripTest(const std::vector<T>& data)
   for (const T& e : data)
   {
     const std::string s = ValueToString(e);
-    T out;
+    T out = T();
     EXPECT_TRUE(TryParse(s, &out));
     EXPECT_EQ(e, out);
   }

Source/UnitTests/Core/PageFaultTest.cpp

@@ -92,8 +92,8 @@ TEST(PageFault, PageFault)
   perform_invalid_access(data);
   auto end = std::chrono::high_resolution_clock::now();
 
-  auto difference_in_nanoseconds = [](auto start, auto end) {
-    return std::chrono::duration_cast<std::chrono::nanoseconds>(end - start).count();
+  auto difference_in_nanoseconds = [](auto from, auto to) {
+    return std::chrono::duration_cast<std::chrono::nanoseconds>(to - from).count();
   };
 
   EMM::UninstallExceptionHandler();