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

@@ -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

src/api/game/game.h

@@ -79,14 +79,14 @@ public:
 private:
   void CacheArchives();
 
+  const GameType type_;
+  const std::filesystem::path gamePath_;
+
   std::shared_ptr<GameCache> cache_;
   std::shared_ptr<LoadOrderHandler> loadOrderHandler_;
   std::shared_ptr<ConditionEvaluator> conditionEvaluator_;
   std::shared_ptr<DatabaseInterface> database_;
 
-  const GameType type_;
-  const std::filesystem::path gamePath_;
-
   std::string masterFilename_;
 };
 }

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<MessageContent>& 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_) {

src/api/metadata/group.cpp

@@ -32,7 +32,7 @@ Group::Group() : name_("default") {}
 Group::Group(const std::string& name,
              const std::vector<std::string>& 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_ &&

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<MessageContent>& 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_) {

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,

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_) {

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,

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<std::remove_pointer<::Plugin>::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<std::string> version_;  // Obtained from description field.
   std::optional<uint32_t> crc_;
   std::vector<Tag> tags_;
-
-  // Useful caches.
-  size_t numOverrideRecords_;
-
-  std::shared_ptr<std::remove_pointer<::Plugin>::type> esPlugin;
 };
 
 std::string GetArchiveFileExtension(const GameType gameType);

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();

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_;

src/tests/api/internals/metadata/condition_evaluator_test.h

@@ -37,10 +37,10 @@ protected:
       info_(std::vector<MessageContent>({
           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));

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)

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<std::string> plugins({

src/tests/common_game_test_fixture.h

@@ -54,9 +54,9 @@ std::filesystem::path getRootTestPath() {
 class CommonGameTestFixture : public ::testing::TestWithParam<GameType> {
 protected:
   CommonGameTestFixture() :
+      rootTestPath(getRootTestPath()),
       french("fr"),
       german("de"),
-      rootTestPath(getRootTestPath()),
       missingPath(rootTestPath / "missing"),
       dataPath(rootTestPath / "game" / getPluginsFolder()),
       localPath(rootTestPath / "local" / "game"),