From 4a4b685a0420b0ac7c026cd2370c23d54f469976 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 30 Oct 2020 15:02:02 -0400 Subject: [PATCH] common: Enable warnings as errors Cleans up common so that we can enable warnings as errors. --- externals/microprofile/microprofile.h | 14 ++++++------ src/common/CMakeLists.txt | 16 ++++++++++++++ src/common/fiber.cpp | 8 +++---- src/common/fiber.h | 2 +- src/common/file_util.cpp | 31 ++++++++++++++++----------- src/common/logging/backend.cpp | 2 -- src/common/string_util.cpp | 5 +++-- src/common/timer.cpp | 12 +++++------ src/common/wall_clock.cpp | 2 +- src/common/x64/native_clock.h | 2 +- 10 files changed, 57 insertions(+), 37 deletions(-) diff --git a/externals/microprofile/microprofile.h b/externals/microprofile/microprofile.h index 85d5bd5dec..5c381f0025 100644 --- a/externals/microprofile/microprofile.h +++ b/externals/microprofile/microprofile.h @@ -902,8 +902,10 @@ inline uint16_t MicroProfileGetGroupIndex(MicroProfileToken t) #include #define snprintf _snprintf +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable: 4244) +#endif int64_t MicroProfileTicksPerSecondCpu() { static int64_t nTicksPerSecond = 0; @@ -946,7 +948,7 @@ typedef HANDLE MicroProfileThread; DWORD _stdcall ThreadTrampoline(void* pFunc) { MicroProfileThreadFunc F = (MicroProfileThreadFunc)pFunc; - return (uint32_t)F(0); + return (DWORD)F(0); } inline void MicroProfileThreadStart(MicroProfileThread* pThread, MicroProfileThreadFunc Func) @@ -1742,10 +1744,10 @@ void MicroProfileFlip() } } } - for(uint32_t i = 0; i < MICROPROFILE_MAX_GROUPS; ++i) + for(uint32_t j = 0; j < MICROPROFILE_MAX_GROUPS; ++j) { - pLog->nGroupTicks[i] += nGroupTicks[i]; - pFrameGroup[i] += nGroupTicks[i]; + pLog->nGroupTicks[j] += nGroupTicks[j]; + pFrameGroup[j] += nGroupTicks[j]; } pLog->nStackPos = nStackPos; } @@ -3328,7 +3330,7 @@ bool MicroProfileIsLocalThread(uint32_t nThreadId) #endif #else -bool MicroProfileIsLocalThread(uint32_t nThreadId){return false;} +bool MicroProfileIsLocalThread([[maybe_unused]] uint32_t nThreadId) { return false; } void MicroProfileStopContextSwitchTrace(){} void MicroProfileStartContextSwitchTrace(){} @@ -3576,7 +3578,7 @@ int MicroProfileGetGpuTickReference(int64_t* pOutCpu, int64_t* pOutGpu) #undef S -#ifdef _WIN32 +#ifdef _MSC_VER #pragma warning(pop) #endif diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index e50ab29222..207c7a0a61 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -190,6 +190,22 @@ if(ARCHITECTURE_x86_64) ) endif() +if (MSVC) + target_compile_definitions(common PRIVATE + # The standard library doesn't provide any replacement for codecvt yet + # so we can disable this deprecation warning for the time being. + _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING + ) + target_compile_options(common PRIVATE + /W4 + /WX + ) +else() + target_compile_options(common PRIVATE + -Werror + ) +endif() + create_target_directory_groups(common) find_package(Boost 1.71 COMPONENTS context headers REQUIRED) diff --git a/src/common/fiber.cpp b/src/common/fiber.cpp index e186ed880c..b209f52fc4 100644 --- a/src/common/fiber.cpp +++ b/src/common/fiber.cpp @@ -79,9 +79,9 @@ void Fiber::Exit() { released = true; } -void Fiber::SetRewindPoint(std::function&& rewind_func, void* start_parameter) { +void Fiber::SetRewindPoint(std::function&& rewind_func, void* rewind_param) { rewind_point = std::move(rewind_func); - rewind_parameter = start_parameter; + rewind_parameter = rewind_param; } void Fiber::Rewind() { @@ -161,9 +161,9 @@ Fiber::Fiber(std::function&& entry_point_func, void* start_paramete boost::context::detail::make_fcontext(stack_base, impl->stack.size(), FiberStartFunc); } -void Fiber::SetRewindPoint(std::function&& rewind_func, void* start_parameter) { +void Fiber::SetRewindPoint(std::function&& rewind_func, void* rewind_param) { rewind_point = std::move(rewind_func); - rewind_parameter = start_parameter; + rewind_parameter = rewind_param; } Fiber::Fiber() : impl{std::make_unique()} {} diff --git a/src/common/fiber.h b/src/common/fiber.h index cefd61df9c..699286ee2b 100644 --- a/src/common/fiber.h +++ b/src/common/fiber.h @@ -49,7 +49,7 @@ public: static void YieldTo(std::shared_ptr from, std::shared_ptr to); [[nodiscard]] static std::shared_ptr ThreadToFiber(); - void SetRewindPoint(std::function&& rewind_func, void* start_parameter); + void SetRewindPoint(std::function&& rewind_func, void* rewind_param); void Rewind(); diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 16c3713e01..18fbfa25bf 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -472,13 +472,14 @@ u64 ScanDirectoryTree(const std::string& directory, FSTEntry& parent_entry, } bool DeleteDirRecursively(const std::string& directory, unsigned int recursion) { - const auto callback = [recursion](u64* num_entries_out, const std::string& directory, - const std::string& virtual_name) -> bool { - std::string new_path = directory + DIR_SEP_CHR + virtual_name; + const auto callback = [recursion](u64*, const std::string& directory, + const std::string& virtual_name) { + const std::string new_path = directory + DIR_SEP_CHR + virtual_name; if (IsDirectory(new_path)) { - if (recursion == 0) + if (recursion == 0) { return false; + } return DeleteDirRecursively(new_path, recursion - 1); } return Delete(new_path); @@ -492,7 +493,8 @@ bool DeleteDirRecursively(const std::string& directory, unsigned int recursion) return true; } -void CopyDir(const std::string& source_path, const std::string& dest_path) { +void CopyDir([[maybe_unused]] const std::string& source_path, + [[maybe_unused]] const std::string& dest_path) { #ifndef _WIN32 if (source_path == dest_path) { return; @@ -553,7 +555,7 @@ std::optional GetCurrentDir() { std::string strDir = dir; #endif free(dir); - return std::move(strDir); + return strDir; } bool SetCurrentDir(const std::string& directory) { @@ -772,21 +774,23 @@ std::size_t ReadFileToString(bool text_file, const std::string& filename, std::s void SplitFilename83(const std::string& filename, std::array& short_name, std::array& extension) { - const std::string forbidden_characters = ".\"/\\[]:;=, "; + static constexpr std::string_view forbidden_characters = ".\"/\\[]:;=, "; // On a FAT32 partition, 8.3 names are stored as a 11 bytes array, filled with spaces. short_name = {{' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '\0'}}; extension = {{' ', ' ', ' ', '\0'}}; - std::string::size_type point = filename.rfind('.'); - if (point == filename.size() - 1) + auto point = filename.rfind('.'); + if (point == filename.size() - 1) { point = filename.rfind('.', point); + } // Get short name. int j = 0; for (char letter : filename.substr(0, point)) { - if (forbidden_characters.find(letter, 0) != std::string::npos) + if (forbidden_characters.find(letter, 0) != std::string::npos) { continue; + } if (j == 8) { // TODO(Link Mauve): also do that for filenames containing a space. // TODO(Link Mauve): handle multiple files having the same short name. @@ -794,14 +798,15 @@ void SplitFilename83(const std::string& filename, std::array& short_nam short_name[7] = '1'; break; } - short_name[j++] = toupper(letter); + short_name[j++] = static_cast(std::toupper(letter)); } // Get extension. if (point != std::string::npos) { j = 0; - for (char letter : filename.substr(point + 1, 3)) - extension[j++] = toupper(letter); + for (char letter : filename.substr(point + 1, 3)) { + extension[j++] = static_cast(std::toupper(letter)); + } } } diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 62cfde3976..90dfa22cad 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -274,7 +274,6 @@ const char* GetLogClassName(Class log_class) { case Class::Count: break; } - UNREACHABLE(); return "Invalid"; } @@ -293,7 +292,6 @@ const char* GetLevelName(Level log_level) { break; } #undef LVL - UNREACHABLE(); return "Invalid"; } diff --git a/src/common/string_util.cpp b/src/common/string_util.cpp index 84883a1d30..4cba2aaa4e 100644 --- a/src/common/string_util.cpp +++ b/src/common/string_util.cpp @@ -8,6 +8,7 @@ #include #include #include + #include "common/common_paths.h" #include "common/logging/log.h" #include "common/string_util.h" @@ -21,14 +22,14 @@ namespace Common { /// Make a string lowercase std::string ToLower(std::string str) { std::transform(str.begin(), str.end(), str.begin(), - [](unsigned char c) { return std::tolower(c); }); + [](unsigned char c) { return static_cast(std::tolower(c)); }); return str; } /// Make a string uppercase std::string ToUpper(std::string str) { std::transform(str.begin(), str.end(), str.begin(), - [](unsigned char c) { return std::toupper(c); }); + [](unsigned char c) { return static_cast(std::toupper(c)); }); return str; } diff --git a/src/common/timer.cpp b/src/common/timer.cpp index 2dc15e4343..d17dc2a503 100644 --- a/src/common/timer.cpp +++ b/src/common/timer.cpp @@ -142,20 +142,18 @@ std::string Timer::GetTimeFormatted() { // ---------------- double Timer::GetDoubleTime() { // Get continuous timestamp - u64 TmpSeconds = static_cast(Common::Timer::GetTimeSinceJan1970().count()); - double ms = static_cast(GetTimeMs().count()) % 1000; + auto tmp_seconds = static_cast(GetTimeSinceJan1970().count()); + const auto ms = static_cast(static_cast(GetTimeMs().count()) % 1000); // Remove a few years. We only really want enough seconds to make // sure that we are detecting actual actions, perhaps 60 seconds is // enough really, but I leave a year of seconds anyway, in case the // user's clock is incorrect or something like that. - TmpSeconds = TmpSeconds - (38 * 365 * 24 * 60 * 60); + tmp_seconds = tmp_seconds - (38 * 365 * 24 * 60 * 60); // Make a smaller integer that fits in the double - u32 Seconds = static_cast(TmpSeconds); - double TmpTime = Seconds + ms; - - return TmpTime; + const auto seconds = static_cast(tmp_seconds); + return seconds + ms; } } // Namespace Common diff --git a/src/common/wall_clock.cpp b/src/common/wall_clock.cpp index 7a20e95b77..452a2837e6 100644 --- a/src/common/wall_clock.cpp +++ b/src/common/wall_clock.cpp @@ -53,7 +53,7 @@ public: return Common::Divide128On32(temporary, 1000000000).first; } - void Pause(bool is_paused) override { + void Pause([[maybe_unused]] bool is_paused) override { // Do nothing in this clock type. } diff --git a/src/common/x64/native_clock.h b/src/common/x64/native_clock.h index 7c503df268..97aab6ac96 100644 --- a/src/common/x64/native_clock.h +++ b/src/common/x64/native_clock.h @@ -34,7 +34,7 @@ private: /// value used to reduce the native clocks accuracy as some apss rely on /// undefined behavior where the level of accuracy in the clock shouldn't /// be higher. - static constexpr u64 inaccuracy_mask = ~(0x400 - 1); + static constexpr u64 inaccuracy_mask = ~(UINT64_C(0x400) - 1); SpinLock rtsc_serialize{}; u64 last_measure{};