From ade9f92a63ca3c2f4146018ce0baacdd36328f77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 21 Nov 2020 00:59:17 +0100 Subject: [PATCH 1/6] Enable extra warnings in Source CMakeLists, not in root CMakeLists Having extra warnings enabled for everything including external libraries produces an overwhelming amount of warnings in code that isn't even part of our codebase. Move the various warning flags to Source/CMakeLists.txt to get rid of those useless warnings. Note that the Source CMakeLists.txt is already where the MSVC warnings are defined, so this commit improves consistency as well. --- CMakeLists.txt | 17 ----------------- Source/CMakeLists.txt | 20 +++++++++++++++++++- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1748ed9347..e67f469c94 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -248,23 +248,6 @@ if(CMAKE_C_COMPILER_ID MATCHES "MSVC") string(APPEND CMAKE_EXE_LINKER_FLAGS " /NXCOMPAT") else() add_definitions(-D_DEFAULT_SOURCE) - check_and_add_flag(HAVE_WALL -Wall) - # 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(INIT_SELF -Winit-self) - check_and_add_flag(MISSING_DECLARATIONS -Wmissing-declarations) - check_and_add_flag(MISSING_VARIABLE_DECLARATIONS -Wmissing-variable-declarations) # gcc uses some optimizations which might break stuff without this flag check_and_add_flag(NO_STRICT_ALIASING -fno-strict-aliasing) diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 015b03f882..df91f4b668 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -62,10 +62,28 @@ if (MSVC) # Use PCH add_subdirectory(PCH) add_definitions(/I${PCH_DIRECTORY}) - add_definitions(/Yu${PCH_PATH}) + add_definitions(/Yu${PCH_PATH}) # Don't include timestamps in binaries add_link_options(/Brepro) +else() + check_and_add_flag(HAVE_WALL -Wall) + # 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(INIT_SELF -Winit-self) + check_and_add_flag(MISSING_DECLARATIONS -Wmissing-declarations) + check_and_add_flag(MISSING_VARIABLE_DECLARATIONS -Wmissing-variable-declarations) endif() # These aren't actually needed for C11/C++11 From 4b7f784d1bcf3a9eb660aaabafaacd80957d2de2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 21 Nov 2020 01:44:15 +0100 Subject: [PATCH 2/6] Disable -Wstringop-truncation warnings 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 or match emulated code that also ignores the null terminator, so the warnings are not useful. Given that Dolphin itself mostly uses std::string, they do not really help catch any bugs. --- Source/CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index df91f4b668..3b70daf4f2 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -84,6 +84,15 @@ else() 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) + + # 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 + # or match emulated code that also ignores the null terminator, so the warnings are not useful. + # Given that Dolphin itself mostly uses std::string, they do not really help catch any bugs. + check_cxx_compiler_flag(-Wstringop-truncation HAS_STRINGOP_TRUNCATION_WARNING) + if (HAS_STRINGOP_TRUNCATION_WARNING) + check_and_add_flag(NO_STRINGOP_TRUNCATION -Wno-stringop-truncation) + endif() endif() # These aren't actually needed for C11/C++11 From 82f1e6204da7fc64a4efecd35dc7e096911c5777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 21 Nov 2020 01:05:01 +0100 Subject: [PATCH 3/6] Fix -Wsign-compare warnings --- Source/Core/Common/Hash.cpp | 2 +- .../Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.cpp | 2 +- .../Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.h | 7 +++++-- Source/Core/DolphinQt/TAS/TASCheckBox.cpp | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Source/Core/Common/Hash.cpp b/Source/Core/Common/Hash.cpp index 395865c475..2f9f47b270 100644 --- a/Source/Core/Common/Hash.cpp +++ b/Source/Core/Common/Hash.cpp @@ -99,7 +99,7 @@ u32 HashEctor(const u8* ptr, size_t length) { u32 crc = 0; - for (int i = 0; i < length; i++) + for (size_t i = 0; i < length; i++) { crc ^= ptr[i]; crc = (crc << 3) | (crc >> 29); diff --git a/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.cpp b/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.cpp index d5020ea8dd..9361547f7b 100644 --- a/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.cpp +++ b/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.cpp @@ -6,7 +6,7 @@ #include "Common/StringUtil.h" -UTF8CodePointCountValidator::UTF8CodePointCountValidator(int max_count, QObject* parent) +UTF8CodePointCountValidator::UTF8CodePointCountValidator(std::size_t max_count, QObject* parent) : QValidator(parent), m_max_count(max_count) { } diff --git a/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.h b/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.h index 89a8bc8e1d..fd952bf56a 100644 --- a/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.h +++ b/Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.h @@ -4,6 +4,8 @@ #pragma once +#include + #include #include @@ -11,9 +13,10 @@ class UTF8CodePointCountValidator : public QValidator { Q_OBJECT public: - explicit UTF8CodePointCountValidator(int max_count, QObject* parent = nullptr); + explicit UTF8CodePointCountValidator(std::size_t max_count, QObject* parent = nullptr); QValidator::State validate(QString& input, int& pos) const override; - int m_max_count; +private: + std::size_t m_max_count; }; diff --git a/Source/Core/DolphinQt/TAS/TASCheckBox.cpp b/Source/Core/DolphinQt/TAS/TASCheckBox.cpp index 8395e7f3f3..c312459696 100644 --- a/Source/Core/DolphinQt/TAS/TASCheckBox.cpp +++ b/Source/Core/DolphinQt/TAS/TASCheckBox.cpp @@ -20,7 +20,7 @@ bool TASCheckBox::GetValue() const if (checkState() == Qt::PartiallyChecked) { const u64 frames_elapsed = Movie::GetCurrentFrame() - m_frame_turbo_started; - return frames_elapsed % m_turbo_total_frames < m_turbo_press_frames; + return static_cast(frames_elapsed % m_turbo_total_frames) < m_turbo_press_frames; } return isChecked(); From 6ab1ab1f120c4441917e334791fd0e171a46e75e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 21 Nov 2020 01:47:45 +0100 Subject: [PATCH 4/6] Fix -Wmaybe-uninitialized warnings --- Source/Core/Core/GeckoCode.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/GeckoCode.h b/Source/Core/Core/GeckoCode.h index 254c65b480..9c6c7c7a14 100644 --- a/Source/Core/Core/GeckoCode.h +++ b/Source/Core/Core/GeckoCode.h @@ -16,7 +16,7 @@ namespace Gecko class GeckoCode { public: - GeckoCode() : enabled(false) {} + GeckoCode() = default; struct Code { u32 address = 0; @@ -28,8 +28,8 @@ public: std::string name, creator; std::vector notes; - bool enabled; - bool user_defined; + bool enabled = false; + bool user_defined = false; bool Exist(u32 address, u32 data) const; }; From 7840f61524768f47ee38c585f62f9e86d0a3d4bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 21 Nov 2020 01:54:44 +0100 Subject: [PATCH 5/6] Silence "missing switch cases" warnings --- Source/Core/Core/IOS/Network/Socket.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/Core/Core/IOS/Network/Socket.cpp b/Source/Core/Core/IOS/Network/Socket.cpp index 184608f090..9de5ff23b1 100644 --- a/Source/Core/Core/IOS/Network/Socket.cpp +++ b/Source/Core/Core/IOS/Network/Socket.cpp @@ -199,6 +199,8 @@ s32 WiiSocket::Shutdown(u32 how) if (shut_write) op.Abort(-SO_ENOTCONN); break; + default: + break; } } return ret; From 9efc81ae98076ce55898ef5634e206d32da713a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Sat, 21 Nov 2020 01:58:22 +0100 Subject: [PATCH 6/6] Fix variable shadowing warnings --- Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp | 6 +++--- Source/Core/InputCommon/ImageOperations.h | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp index 39b989d141..a31384aee4 100644 --- a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp +++ b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp @@ -1049,10 +1049,10 @@ NetPlayDialog::FindGameFile(const NetPlay::SyncIdentifier& sync_identifier, RunOnObject(this, [this, &sync_identifier, found] { for (int i = 0; i < m_game_list_model.rowCount(QModelIndex()); i++) { - auto game_file = m_game_list_model.GetGameFile(i); - *found = std::min(*found, game_file->CompareSyncIdentifier(sync_identifier)); + auto file = m_game_list_model.GetGameFile(i); + *found = std::min(*found, file->CompareSyncIdentifier(sync_identifier)); if (*found == NetPlay::SyncIdentifierComparison::SameGame) - return game_file; + return file; } return static_cast>(nullptr); }); diff --git a/Source/Core/InputCommon/ImageOperations.h b/Source/Core/InputCommon/ImageOperations.h index 28d3859ff5..87a4b37d6d 100644 --- a/Source/Core/InputCommon/ImageOperations.h +++ b/Source/Core/InputCommon/ImageOperations.h @@ -32,13 +32,13 @@ struct ImagePixelData { ImagePixelData() = default; - explicit ImagePixelData(std::vector image_pixels, u32 width, u32 height) - : pixels(std::move(image_pixels)), width(width), height(height) + explicit ImagePixelData(std::vector image_pixels, u32 width_, u32 height_) + : pixels(std::move(image_pixels)), width(width_), height(height_) { } - explicit ImagePixelData(u32 width, u32 height, const Pixel& default_color = Pixel{0, 0, 0, 0}) - : pixels(width * height, default_color), width(width), height(height) + explicit ImagePixelData(u32 width_, u32 height_, const Pixel& default_color = Pixel{0, 0, 0, 0}) + : pixels(width_ * height_, default_color), width(width_), height(height_) { } std::vector pixels;