[MU4] Fix compiler warnings

[Jojo-Schmitz]

trafficstars

• MSVC: reg. conversion from 'size_t' to 'int', possible loss of data (C4267).
• MinGW: reg. comparison of integer expressions of different signedness [-Wsign-compare].
• MSVC: reg. cast truncates constant value (C4310).
• MSVC: reg. "the usage of 'method' requires the compiler to capture 'this' but the current default capture mode does not allow it (C4573)", by disabling it.
• MSVC: reg. "unreferenced function with internal linkage has been removed", by disabling them resp. removing the code.
• MSCV/MinGW: reg. unreferenced formal parameter (C4100) resp. unused parameter [-Wunused-parameter].
• MinGW: reg. this statement may fall through [-Wimplicit-fallthrough=] (at least one of which is a bug!).
• MSVC: reg. unreachable code (C4702).
• MinGW: reg. suggest parentheses around '&&' within '||' [-Wparentheses].
• MinGW: reg. variable set but not used [-Wunused-but-set-variable].
• MinGW: reg. suggest parentheses around assignment used as truth value [-Wparentheses].
• MSVC: reg. declaration hides global declaration (C4459).
• MSVC: reg. declaration hides function parameter (C4457).
• MSVC: reg. conversion requires a narrowing conversion (C4838)
• MSVC: reg. "structure was padded due to alignment specifier (C4324)", by disabling it
• GitHub CI: reg. Node.js 12 actions being deprecated vs. 16
• GitHub CI: reg. set-output command being deprecated
• XCode: reg. loop variable is always a copy because the range of type 'QJsonArray' does not return a reference [-Wrange-loop-analysis]
• MSVC: reg. local variable is initialized but not referenced (C4189)

[Jojo-Schmitz]

There's a new MSVC compiler I don't know how to handle:

Severity	Code	Description	Project	File	Line	Suppression State
Warning	C4573	the usage of 'QObject::connect' requires the compiler to capture 'this' but the current default capture mode does not allow it	plugins_tests	...\src\plugins\tests\environment.cpp	47	

[cbjeukendrup]

Wow. That looks like a MSVC quirk again! this is not even meaningful in that context.

Anyway, maybe this works?

-    QObject::connect(qmlEngine, &QQmlEngine::warnings, [](const QList<QQmlError>& warnings) {
+    QObject::connect(qmlEngine, &QQmlEngine::warnings, [=](const QList<QQmlError>& warnings) {

[jeetee]

I'd rather expect you'd need the [=] on line 38 instead, as QObject::connect is not a static method?

[cbjeukendrup]

Probably you're right. But QObject::connect is surely static here, but there is one overload that is non-static. That probably confuses MSVC. We have seen that before, that time with disconnect instead of connect.

[Jojo-Schmitz]

Wow. That looks like a MSVC quirk again! this is not even meaningful in that context.

Anyway, maybe this works?

-    QObject::connect(qmlEngine, &QQmlEngine::warnings, [](const QList<QQmlError>& warnings) {
+    QObject::connect(qmlEngine, &QQmlEngine::warnings, [=](const QList<QQmlError>& warnings) {

No, doesn't help, still the same warning

I'd rather expect you'd need the [=] on line 38 instead, as QObject::connect is not a static method?

No, doesn't even compile!

[jeetee]

Probably you're right. But QObject::connect is surely static here,

The static call should perhaps have the receiver parameter for MSVC to not confuse it with the method signature? Perhaps try QObject::connect(qmlEngine, &QQmlEngine::warnings, this, [=](const QList<QQmlError>& warnings) { given that is what the member-connect is supposed to expand to? (https://doc.qt.io/qt-5/qobject.html#connect-2)

EDIT: and drop the capture indication as the body doesn't need it: QObject::connect(qmlEngine, &QQmlEngine::warnings, this, [](const QList<QQmlError>& warnings) {

[Jojo-Schmitz]

Probably you're right. But QObject::connect is surely static here,

The static call should perhaps have the receiver parameter for MSVC to not confuse it with the method signature? Perhaps try QObject::connect(qmlEngine, &QQmlEngine::warnings, this, [=](const QList<QQmlError>& warnings) { given that is what the member-connect is supposed to expand to? (https://doc.qt.io/qt-5/qobject.html#connect-2)

EDIT: and drop the capture indication as the body doesn't need it: QObject::connect(qmlEngine, &QQmlEngine::warnings, this, [](const QList<QQmlError>& warnings) {

Neither compiles

[cbjeukendrup]

@Jojo-Schmitz In https://developercommunity.visualstudio.com/t/bogus-warning-c4573-for-static-method-with-same-na/312074, they claim that this MSVC bug should have been fixed. But apparently not for us...

Can we perhaps just disable the warning, at least for that file or those few lines?

[Jojo-Schmitz]

OK, let's disable it Even MSVC 2022 cvomplains about it.

[Jojo-Schmitz]

More and more warnings are ramping up, I'd appreciate a review and merge...

[Jojo-Schmitz]

There are 3 new warnings: src\framework\audio\internal/audiobuffer.h(62,63,64): warning C4324: 'mu::audio::AudioBuffer': structure was padded due to alignment specifier No idea how to fix those

[cbjeukendrup]

MSVC complains that we're making those structs/members bigger than their intrinsic size by specifying alignment. But in this case, that is exactly what we want to do; we want them to be in different memory cache lines, and therefore they must be padded. So we should just disable these warnings.

[Jojo-Schmitz]

OK, it is disabled now ;-)

[Jojo-Schmitz]

Hmm, what on earth is now causing that unit test failure at https://github.com/musescore/MuseScore/actions/runs/3872958358/jobs/6602418643#step:9:1537 ? As far as I can tell I did nothing but a rebase.

[cbjeukendrup]

Seems related to my work on the tests; that test seems to fail sometimes, and sometimes not...

[Jojo-Schmitz]

Seems related to my work on the tests; that test seems to fail sometimes, and sometimes not...

There's nothing like reliable tests ;-)

[Jojo-Schmitz]

OK, tests worked this time