fix: more MSVC warnings (#5137)

CHANGELOG.md

@@ -123,7 +123,7 @@
 - Dev: Load less message history upon reconnects. (#5001, #5018)
 - Dev: Removed the `NullablePtr` class. (#5091)
 - Dev: BREAKING: Replace custom `import()` with normal Lua `require()`. (#5014, #5108)
-- Dev: Fixed most compiler warnings. (#5028)
+- Dev: Fixed most compiler warnings. (#5028, #5137)
 - Dev: Added the ability to show `ChannelView`s without a `Split`. (#4747)
 - Dev: Refactor Args to be less of a singleton. (#5041)
 - Dev: Channels without any animated elements on screen will skip updates from the GIF timer. (#5042, #5043, #5045)

src/CMakeLists.txt

@@ -935,6 +935,7 @@ target_compile_definitions(${LIBRARY_PROJECT} PUBLIC
     AB_CUSTOM_SETTINGS
     IRC_STATIC
     IRC_NAMESPACE=Communi
+    $<$<BOOL:${WIN32}>:_WIN32_WINNT=0x0A00> # Windows 10
     )
 
 if (USE_SYSTEM_QTKEYCHAIN)
@@ -1048,10 +1049,6 @@ if (MSVC)
     # Someone adds /W3 before we add /W4.
     # This makes sure, only /W4 is specified.
     string(REPLACE "/W3" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-    # 4505 - "unreferenced local version has been removed"
-    #        Although this might give hints on dead code,
-    #        there are some cases where it's distracting.
-    #
     # 4100 - "unreferenced formal parameter"
     #        There are a lot of functions and methods where
     #        an argument was given a name but never used.
@@ -1062,6 +1059,11 @@ if (MSVC)
     #        These are implicit conversions from size_t to int/qsizetype.
     #        We don't use size_t in a lot of cases, since
     #        Qt doesn't use it - it uses int (or qsizetype in Qt6).
+    #
+    # 4458 - "declaration of 'identifier' hides class member"
+    #        We have a rule of exclusively using `this->`
+    #        to access class members, thus it's fine to reclare a variable
+    #        with the same name as a class member.
     target_compile_options(${LIBRARY_PROJECT} PUBLIC
         /W4
         # 5038 - warnings about initialization order
@@ -1069,9 +1071,9 @@ if (MSVC)
         # 4855 - implicit capture of 'this' via '[=]' is deprecated
         /w14855
         # Disable the following warnings (see reasoning above)
-        /wd4505
         /wd4100
         /wd4267
+        /wd4458
         # Enable updated '__cplusplus' macro - workaround for CMake#18837
         /Zc:__cplusplus
     )

src/common/Channel.cpp

@@ -82,7 +82,6 @@ LimitedQueueSnapshot<MessagePtr> Channel::getMessageSnapshot()
 void Channel::addMessage(MessagePtr message,
                          std::optional<MessageFlags> overridingFlags)
 {
-    auto *app = getApp();
     MessagePtr deleted;
 
     if (!overridingFlags || !overridingFlags->has(MessageFlag::DoNotLog))

src/common/Credentials.cpp

@@ -1,6 +1,7 @@
 #include "common/Credentials.hpp"
 
 #include "Application.hpp"
+#include "common/Modes.hpp"
 #include "debug/AssertInGuiThread.hpp"
 #include "singletons/Paths.hpp"
 #include "singletons/Settings.hpp"
@@ -41,7 +42,7 @@ bool useKeyring()
 #ifdef NO_QTKEYCHAIN
     return false;
 #endif
-    if (getIApp()->getPaths().isPortable())
+    if (Modes::instance().isPortable)
     {
         return false;
     }

src/debug/AssertInGuiThread.hpp

@@ -7,12 +7,12 @@
 
 namespace chatterino {
 
-static bool isGuiThread()
+inline bool isGuiThread()
 {
     return QCoreApplication::instance()->thread() == QThread::currentThread();
 }
 
-static void assertInGuiThread()
+inline void assertInGuiThread()
 {
 #ifdef _DEBUG
     assert(isGuiThread());

src/messages/layouts/MessageLayoutElement.cpp

@@ -504,7 +504,7 @@ int TextLayoutElement::getXFromIndex(size_t index)
     {
         return this->getRect().left();
     }
-    else if (index < this->getText().size())
+    else if (index < static_cast<size_t>(this->getText().size()))
     {
         int x = 0;
         for (int i = 0; i < index; i++)

src/providers/seventv/eventapi/Message.cpp

@@ -8,4 +8,16 @@ Message::Message(QJsonObject _json)
 {
 }
 
+std::optional<Message> parseBaseMessage(const QString &blob)
+{
+    QJsonDocument jsonDoc(QJsonDocument::fromJson(blob.toUtf8()));
+
+    if (jsonDoc.isNull())
+    {
+        return std::nullopt;
+    }
+
+    return Message(jsonDoc.object());
+}
+
 }  // namespace chatterino::seventv::eventapi

src/providers/seventv/eventapi/Message.hpp

@@ -28,16 +28,6 @@ std::optional<InnerClass> Message::toInner()
     return InnerClass{this->data};
 }
 
-static std::optional<Message> parseBaseMessage(const QString &blob)
-{
-    QJsonDocument jsonDoc(QJsonDocument::fromJson(blob.toUtf8()));
-
-    if (jsonDoc.isNull())
-    {
-        return std::nullopt;
-    }
-
-    return Message(jsonDoc.object());
-}
+std::optional<Message> parseBaseMessage(const QString &blob);
 
 }  // namespace chatterino::seventv::eventapi

src/providers/twitch/api/Helix.cpp

@@ -2703,8 +2703,12 @@ void Helix::updateShieldMode(
                                            Qt::CaseInsensitive))
                     {
                         failureCallback(Error::UserMissingScope, message);
+                        break;
                     }
+
+                    failureCallback(Error::Forwarded, message);
                 }
+                break;
                 case 401: {
                     failureCallback(Error::Forwarded, message);
                 }

src/providers/twitch/pubsubmessages/Base.cpp

@@ -16,4 +16,16 @@ PubSubMessage::PubSubMessage(QJsonObject _object)
     }
 }
 
+std::optional<PubSubMessage> parsePubSubBaseMessage(const QString &blob)
+{
+    QJsonDocument jsonDoc(QJsonDocument::fromJson(blob.toUtf8()));
+
+    if (jsonDoc.isNull())
+    {
+        return std::nullopt;
+    }
+
+    return PubSubMessage(jsonDoc.object());
+}
+
 }  // namespace chatterino

src/providers/twitch/pubsubmessages/Base.hpp

@@ -45,17 +45,7 @@ std::optional<InnerClass> PubSubMessage::toInner()
     return InnerClass{this->nonce, data};
 }
 
-static std::optional<PubSubMessage> parsePubSubBaseMessage(const QString &blob)
-{
-    QJsonDocument jsonDoc(QJsonDocument::fromJson(blob.toUtf8()));
-
-    if (jsonDoc.isNull())
-    {
-        return std::nullopt;
-    }
-
-    return PubSubMessage(jsonDoc.object());
-}
+std::optional<PubSubMessage> parsePubSubBaseMessage(const QString &blob);
 
 }  // namespace chatterino
 

src/singletons/NativeMessaging.cpp

@@ -2,6 +2,7 @@
 
 #include "Application.hpp"
 #include "common/Literals.hpp"
+#include "common/Modes.hpp"
 #include "common/QLogging.hpp"
 #include "debug/AssertInGuiThread.hpp"
 #include "providers/twitch/TwitchIrcServer.hpp"
@@ -40,7 +41,7 @@ void registerNmManifest(const Paths &paths, const QString &manifestFilename,
 
 void registerNmHost(const Paths &paths)
 {
-    if (paths.isPortable())
+    if (Modes::instance().isPortable)
     {
         return;
     }

src/singletons/Paths.cpp

@@ -86,7 +86,7 @@ void Paths::initRootDirectory()
 
     this->rootAppDataDirectory = [&]() -> QString {
         // portable
-        if (this->isPortable())
+        if (Modes::instance().isPortable)
         {
             return QCoreApplication::applicationDirPath();
         }

src/singletons/Updates.cpp

@@ -90,7 +90,7 @@ void Updates::installUpdates()
     box->exec();
     QDesktopServices::openUrl(this->updateGuideLink_);
 #elif defined Q_OS_WIN
-    if (this->paths.isPortable())
+    if (Modes::instance().isPortable)
     {
         QMessageBox *box =
             new QMessageBox(QMessageBox::Information, "Chatterino Update",

src/util/StandardItemHelper.hpp

@@ -5,7 +5,7 @@
 
 namespace chatterino {
 
-static auto defaultItemFlags(bool selectable)
+inline auto defaultItemFlags(bool selectable)
 {
     return Qt::ItemIsEnabled |
            (selectable ? Qt::ItemIsSelectable | Qt::ItemIsDragEnabled |
@@ -13,7 +13,7 @@ static auto defaultItemFlags(bool selectable)
                        : Qt::ItemFlag());
 }
 
-static void setBoolItem(QStandardItem *item, bool value,
+inline void setBoolItem(QStandardItem *item, bool value,
                         bool userCheckable = true, bool selectable = true)
 {
     item->setFlags(
@@ -22,7 +22,7 @@ static void setBoolItem(QStandardItem *item, bool value,
     item->setCheckState(value ? Qt::Checked : Qt::Unchecked);
 }
 
-static void setStringItem(QStandardItem *item, const QString &value,
+inline void setStringItem(QStandardItem *item, const QString &value,
                           bool editable = true, bool selectable = true)
 {
     item->setData(value, Qt::EditRole);
@@ -30,7 +30,7 @@ static void setStringItem(QStandardItem *item, const QString &value,
                                  (editable ? (Qt::ItemIsEditable) : 0)));
 }
 
-static void setFilePathItem(QStandardItem *item, const QUrl &value,
+inline void setFilePathItem(QStandardItem *item, const QUrl &value,
                             bool selectable = true)
 {
     item->setData(value, Qt::UserRole);
@@ -40,7 +40,7 @@ static void setFilePathItem(QStandardItem *item, const QUrl &value,
                       (selectable ? Qt::ItemIsSelectable : Qt::NoItemFlags)));
 }
 
-static void setColorItem(QStandardItem *item, const QColor &value,
+inline void setColorItem(QStandardItem *item, const QColor &value,
                          bool selectable = true)
 {
     item->setData(value, Qt::DecorationRole);
@@ -49,7 +49,7 @@ static void setColorItem(QStandardItem *item, const QColor &value,
                       (selectable ? Qt::ItemIsSelectable : Qt::NoItemFlags)));
 }
 
-static QStandardItem *emptyItem()
+inline QStandardItem *emptyItem()
 {
     auto *item = new QStandardItem();
     item->setFlags(Qt::ItemFlags());

src/widgets/settingspages/GeneralPageView.hpp

@@ -220,9 +220,9 @@ public:
     {
         auto *combo = this->addDropdown(text, {}, std::move(toolTipText));
 
-        for (const auto &[text, userData] : items)
+        for (const auto &[itemText, userData] : items)
         {
-            combo->addItem(text, userData);
+            combo->addItem(itemText, userData);
         }
 
         if (!defaultValueText.isEmpty())

src/widgets/splits/SplitContainer.cpp

@@ -869,10 +869,11 @@ void SplitContainer::applyFromDescriptorRecursively(
                 auto *node = new Node();
                 node->parent_ = baseNode;
 
-                if (const auto *n = std::get_if<ContainerNodeDescriptor>(&item))
+                if (const auto *inner =
+                        std::get_if<ContainerNodeDescriptor>(&item))
                 {
-                    node->flexH_ = n->flexH_;
-                    node->flexV_ = n->flexV_;
+                    node->flexH_ = inner->flexH_;
+                    node->flexV_ = inner->flexV_;
                 }
 
                 baseNode->children_.emplace_back(node);