From 82d09ae9a49685220a4ca48c7c9a12a0d593e196 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sun, 6 Feb 2022 19:57:55 +0000 Subject: [PATCH] Add -Wall, -Wextra to GCC compile options Fix the warnings that get displayed. They're almost all due to misleading member initialisation orders. --- CMakeLists.txt | 24 ++++++++++++++----- src/api/game/game.h | 6 ++--- src/api/metadata/file.cpp | 4 ++-- src/api/metadata/group.cpp | 2 +- src/api/metadata/message.cpp | 10 ++++---- src/api/metadata/plugin_cleaning_data.cpp | 2 +- src/api/metadata/tag.cpp | 2 +- src/api/plugin.cpp | 4 ++-- src/api/plugin.h | 9 +++---- .../api/interface/database_interface_test.h | 6 ++--- .../api/internals/game/game_cache_test.h | 4 ++-- .../metadata/condition_evaluator_test.h | 6 ++--- src/tests/api/internals/plugin_test.h | 8 +++---- .../api/internals/sorting/plugin_sort_test.h | 4 ++-- src/tests/common_game_test_fixture.h | 2 +- 15 files changed, 51 insertions(+), 42 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7f7e4ca2..82a75454 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -384,9 +384,11 @@ target_link_libraries(libloot_tests PRIVATE loot ${GTEST_LIBRARIES}) # Set Target-Specific Flags ############################## -set(LIBLOOT_COMMON_INCLUDE_DIRS +set(LIBLOOT_INCLUDE_DIRS "${CMAKE_SOURCE_DIR}/src" - "${CMAKE_SOURCE_DIR}/include" + "${CMAKE_SOURCE_DIR}/include") + +set(LIBLOOT_COMMON_SYSTEM_INCLUDE_DIRS ${LIBLOADORDER_INCLUDE_DIRS} ${ESPLUGIN_INCLUDE_DIRS} ${LCI_INCLUDE_DIRS} @@ -396,13 +398,19 @@ set(LIBLOOT_COMMON_INCLUDE_DIRS ${YAML_CPP_INCLUDE_DIRS}) target_include_directories(libloot_internals_tests PRIVATE - ${LIBLOOT_COMMON_INCLUDE_DIRS} + ${LIBLOOT_INCLUDE_DIRS}) + +target_include_directories(libloot_internals_tests SYSTEM PRIVATE + ${LIBLOOT_COMMON_SYSTEM_INCLUDE_DIRS} ${GTEST_INCLUDE_DIRS}) -target_include_directories(loot PRIVATE ${LIBLOOT_COMMON_INCLUDE_DIRS}) +target_include_directories(loot PRIVATE ${LIBLOOT_INCLUDE_DIRS}) +target_include_directories(loot SYSTEM PRIVATE + ${LIBLOOT_COMMON_SYSTEM_INCLUDE_DIRS}) -target_include_directories(libloot_tests PRIVATE - ${LIBLOOT_COMMON_INCLUDE_DIRS} +target_include_directories(libloot_tests PRIVATE ${LIBLOOT_INCLUDE_DIRS}) +target_include_directories(libloot_tests SYSTEM PRIVATE + ${LIBLOOT_COMMON_SYSTEM_INCLUDE_DIRS} ${GTEST_INCLUDE_DIRS}) if(CMAKE_SYSTEM_NAME STREQUAL "Windows") @@ -421,6 +429,10 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") INSTALL_RPATH "${CMAKE_INSTALL_RPATH};." BUILD_WITH_INSTALL_RPATH ON) + target_compile_options(libloot_internals_tests PRIVATE "-Wall" "-Wextra") + target_compile_options(loot PRIVATE "-Wall" "-Wextra") + target_compile_options(libloot_tests PRIVATE "-Wall" "-Wextra") + set(LOOT_LIBS ssl curl z diff --git a/src/api/game/game.h b/src/api/game/game.h index e9a73194..5726aa97 100644 --- a/src/api/game/game.h +++ b/src/api/game/game.h @@ -79,14 +79,14 @@ public: private: void CacheArchives(); + const GameType type_; + const std::filesystem::path gamePath_; + std::shared_ptr cache_; std::shared_ptr loadOrderHandler_; std::shared_ptr conditionEvaluator_; std::shared_ptr database_; - const GameType type_; - const std::filesystem::path gamePath_; - std::string masterFilename_; }; } diff --git a/src/api/metadata/file.cpp b/src/api/metadata/file.cpp index 4cc325aa..b2e3596e 100644 --- a/src/api/metadata/file.cpp +++ b/src/api/metadata/file.cpp @@ -34,10 +34,10 @@ File::File(const std::string& name, const std::string& display, const std::string& condition, const std::vector& detail) : + ConditionalMetadata(condition), name_(Filename(name)), display_(display), - detail_(detail), - ConditionalMetadata(condition) {} + detail_(detail) {} bool File::operator<(const File& rhs) const { if (display_ < rhs.display_) { diff --git a/src/api/metadata/group.cpp b/src/api/metadata/group.cpp index cb716cbf..a9daca98 100644 --- a/src/api/metadata/group.cpp +++ b/src/api/metadata/group.cpp @@ -32,7 +32,7 @@ Group::Group() : name_("default") {} Group::Group(const std::string& name, const std::vector& afterGroups, const std::string& description) : - name_(name), afterGroups_(afterGroups), description_(description) {} + name_(name), description_(description), afterGroups_(afterGroups) {} bool Group::operator==(const Group& rhs) const { return name_ == rhs.name_ && description_ == rhs.description_ && diff --git a/src/api/metadata/message.cpp b/src/api/metadata/message.cpp index 04356ffc..0a4cc0d0 100644 --- a/src/api/metadata/message.cpp +++ b/src/api/metadata/message.cpp @@ -34,14 +34,14 @@ Message::Message() : type_(MessageType::say) {} Message::Message(const MessageType type, const std::string& content, const std::string& condition) : + ConditionalMetadata(condition), type_(type), - content_({MessageContent(content)}), - ConditionalMetadata(condition) {} + content_({MessageContent(content)}) {} Message::Message(const MessageType type, const std::vector& content, const std::string& condition) : - type_(type), content_(content), ConditionalMetadata(condition) { + ConditionalMetadata(condition), type_(type), content_(content) { if (content.size() > 1) { bool englishStringExists = false; for (const auto& mc : content) { @@ -56,9 +56,9 @@ Message::Message(const MessageType type, } Message::Message(const SimpleMessage& message) : + ConditionalMetadata(message.condition), type_(message.type), - content_({MessageContent(message.text, message.language)}), - ConditionalMetadata(message.condition) {} + content_({MessageContent(message.text, message.language)}) {} bool Message::operator<(const Message& rhs) const { if (type_ < rhs.type_) { diff --git a/src/api/metadata/plugin_cleaning_data.cpp b/src/api/metadata/plugin_cleaning_data.cpp index a482a065..45eade1c 100644 --- a/src/api/metadata/plugin_cleaning_data.cpp +++ b/src/api/metadata/plugin_cleaning_data.cpp @@ -33,7 +33,7 @@ PluginCleaningData::PluginCleaningData() : crc_(0), itm_(0), ref_(0), nav_(0) {} PluginCleaningData::PluginCleaningData(uint32_t crc, const std::string& utility) : - crc_(crc), utility_(utility), itm_(0), ref_(0), nav_(0) {} + crc_(crc), itm_(0), ref_(0), nav_(0), utility_(utility) {} PluginCleaningData::PluginCleaningData( uint32_t crc, diff --git a/src/api/metadata/tag.cpp b/src/api/metadata/tag.cpp index 7b46122a..4c7f4e80 100644 --- a/src/api/metadata/tag.cpp +++ b/src/api/metadata/tag.cpp @@ -32,7 +32,7 @@ Tag::Tag() : addTag_(true) {} Tag::Tag(const std::string& tag, const bool isAddition, const std::string& condition) : - name_(tag), addTag_(isAddition), ConditionalMetadata(condition) {} + ConditionalMetadata(condition), name_(tag), addTag_(isAddition) {} bool Tag::operator<(const Tag& rhs) const { if (addTag_ != rhs.addTag_) { diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index dac9cafd..899b39f1 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -334,11 +334,11 @@ bool equivalent(const std::filesystem::path& path1, // so check with the filesystem. try { return std::filesystem::equivalent(path1, path2); - } catch (std::filesystem::filesystem_error) { + } catch (const std::filesystem::filesystem_error&) { // One of the paths checked for equivalence doesn't exist, // so they can't be equivalent. return false; - } catch (std::system_error) { + } catch (const std::system_error&) { // This can be thrown if one or both of the paths contains a character // that can't be represented in Windows' multi-byte code page (e.g. // Windows-1252), even though Unicode paths shouldn't be a problem, diff --git a/src/api/plugin.h b/src/api/plugin.h index 5665cbf8..7d58cf79 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -85,18 +85,15 @@ private: const std::filesystem::path& pluginPath); static unsigned int GetEspluginGameId(GameType gameType); + const std::string name_; + std::shared_ptr::type> esPlugin; bool isEmpty_; // Does the plugin contain any records other than the TES4 // header? bool loadsArchive_; - const std::string name_; + size_t numOverrideRecords_; std::optional version_; // Obtained from description field. std::optional crc_; std::vector tags_; - - // Useful caches. - size_t numOverrideRecords_; - - std::shared_ptr::type> esPlugin; }; std::string GetArchiveFileExtension(const GameType gameType); diff --git a/src/tests/api/interface/database_interface_test.h b/src/tests/api/interface/database_interface_test.h index f662b022..05328a7a 100644 --- a/src/tests/api/interface/database_interface_test.h +++ b/src/tests/api/interface/database_interface_test.h @@ -33,13 +33,13 @@ namespace test { class DatabaseInterfaceTest : public ApiGameOperationsTest { protected: DatabaseInterfaceTest() : - db_(nullptr), userlistPath_(localPath / "userlist.yaml"), + minimalOutputPath_(localPath / "minimal.yml"), url_("./testing-metadata.git"), branch_("master"), oldBranch_("old-branch"), - minimalOutputPath_(localPath / "minimal.yml"), - generalUserlistMessage("A general userlist message.") {} + generalUserlistMessage("A general userlist message."), + db_(nullptr) {} void SetUp() { ApiGameOperationsTest::SetUp(); diff --git a/src/tests/api/internals/game/game_cache_test.h b/src/tests/api/internals/game/game_cache_test.h index f9f2c2a4..db45c00d 100644 --- a/src/tests/api/internals/game/game_cache_test.h +++ b/src/tests/api/internals/game/game_cache_test.h @@ -34,9 +34,9 @@ namespace test { class GameCacheTest : public CommonGameTestFixture { protected: GameCacheTest() : + game_(GetParam(), dataPath.parent_path(), localPath), condition("Condition"), - conditionLowercase("condition"), - game_(GetParam(), dataPath.parent_path(), localPath) {} + conditionLowercase("condition") {} Game game_; GameCache cache_; diff --git a/src/tests/api/internals/metadata/condition_evaluator_test.h b/src/tests/api/internals/metadata/condition_evaluator_test.h index bd8202c0..90767ab7 100644 --- a/src/tests/api/internals/metadata/condition_evaluator_test.h +++ b/src/tests/api/internals/metadata/condition_evaluator_test.h @@ -37,10 +37,10 @@ protected: info_(std::vector({ MessageContent("info"), })), - game_(GetParam(), dataPath.parent_path(), localPath), - evaluator_(game_.Type(), game_.DataPath()), nonAsciiEsm(u8"non\u00C1scii.esm"), - nonAsciiNestedFile(u8"non\u00C1scii/test.txt") { + nonAsciiNestedFile(u8"non\u00C1scii/test.txt"), + game_(GetParam(), dataPath.parent_path(), localPath), + evaluator_(game_.Type(), game_.DataPath()) { // Make sure the plugin with a non-ASCII filename exists. std::filesystem::copy_file(dataPath / blankEsm, dataPath / std::filesystem::u8path(nonAsciiEsm)); diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index 90845ee0..75b3731d 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -38,10 +38,10 @@ protected: lowercaseBlankEsp("blank.esp"), nonAsciiEsp(u8"non\u00C1scii.esp"), otherNonAsciiEsp(u8"other non\u00C1scii.esp"), - game_(GetParam(), dataPath.parent_path(), localPath), blankArchive("Blank" + GetArchiveFileExtension(game_.Type())), blankSuffixArchive("Blank - Different - suffix" + - GetArchiveFileExtension(game_.Type())) {} + GetArchiveFileExtension(game_.Type())), + game_(GetParam(), dataPath.parent_path(), localPath) {} void SetUp() { CommonGameTestFixture::SetUp(); @@ -118,8 +118,6 @@ protected: } } - Game game_; - const std::string emptyFile; const std::string lowercaseBlankEsp; const std::string nonAsciiEsp; @@ -127,6 +125,8 @@ protected: const std::string blankArchive; const std::string blankSuffixArchive; + Game game_; + private: static std::string GetArchiveFileExtension(const GameType gameType) { if (gameType == GameType::fo4) diff --git a/src/tests/api/internals/sorting/plugin_sort_test.h b/src/tests/api/internals/sorting/plugin_sort_test.h index 09d19c24..03a257d0 100644 --- a/src/tests/api/internals/sorting/plugin_sort_test.h +++ b/src/tests/api/internals/sorting/plugin_sort_test.h @@ -36,9 +36,9 @@ class PluginSortTest : public CommonGameTestFixture { protected: PluginSortTest() : game_(GetParam(), dataPath.parent_path(), localPath), + blankEslEsp("Blank.esl.esp"), masterlistPath_(metadataFilesPath / "userlist.yaml"), - cccPath_(dataPath.parent_path() / getCCCFilename()), - blankEslEsp("Blank.esl.esp") {} + cccPath_(dataPath.parent_path() / getCCCFilename()) {} void loadInstalledPlugins(Game &game, bool headersOnly) { std::vector plugins({ diff --git a/src/tests/common_game_test_fixture.h b/src/tests/common_game_test_fixture.h index cf3b35f7..5f91ecf7 100644 --- a/src/tests/common_game_test_fixture.h +++ b/src/tests/common_game_test_fixture.h @@ -54,9 +54,9 @@ std::filesystem::path getRootTestPath() { class CommonGameTestFixture : public ::testing::TestWithParam { protected: CommonGameTestFixture() : + rootTestPath(getRootTestPath()), french("fr"), german("de"), - rootTestPath(getRootTestPath()), missingPath(rootTestPath / "missing"), dataPath(rootTestPath / "game" / getPluginsFolder()), localPath(rootTestPath / "local" / "game"),