MSVC warnings need to be fixed

• Aug 13, 2018 - 10:00
Reported version
3.0
Type
Functional
Severity
S3 - Major
Status
closed
Regression
No
Workaround
No
Project
MuseScore
Tags
3.0alpha
msvc
vs2017

There is a list of warnings we have (with /W4 level) in MSVC environment:

  • Warning C4065 switch statement contains 'default' but no 'case' label
  • Warning C4121 'Operator': alignment of a member was sensitive to packing
  • Warning C4127 conditional expression is constant
  • Warning C4245 '=': conversion from '' to '', signed/unsigned mismatch
  • Warning C4456 declaration of '' hides previous local declaration
  • Warning C4458 declaration of '' hides class member
  • Warning C4702 unreachable code
  • Warning C4706 assignment within conditional expression
  • Warning C4996 'strdup': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name

Comments

• Aug 13, 2018 - 10:06

Some warnings fixed in https://github.com/musescore/MuseScore/pull/3858:

  • Warnings C4065
  • Warnings C4244
  • Warnings C4304
  • Warnings C4456 (some, those due to nested foreach loops).
  • Warnings C4457
  • Warnings C4459
  • Warnings C4701
  • Warnings C4702 (all but 2, see below).
  • Warnings C4804
  • Warnings C4838

Left to do:

  • Those two remaining C4702 warnings (unreachable code), in mscore/importgtp-gp4.cpp (lines 1023 and 1082 (both understood now, but no idea how to fix)).
  • A whole bunch (406!) of (remaining) C4456 and C4458 warnings (variables shadowing others, either local or class members). These are relativly easy to fix, it is just a lot of work, and some guidance on preferred (re-)naming is needed.

Haven't see any of Warnings C4121, C4127, C4245, C4706 or C4996 though yet?

Edit:
Warning С4127 and C4706 have been already fixed.
Warning C4245, C4121 and C4996 relate to the thirparty component poppler and have been disabled meanwhile.
Warning C4121 and C4996 had been seen once but now don't show anymore.

Reply

• Aug 14, 2018 - 11:03
Severity ⇒
Status (old) patch (code needs review) ⇒ fixed
Status ⇒ fixed

Fixed in branch master, commit 632f830f5f

Fix #275218: fix MSVC compiler warnings

* Warnings C4304, C4804 and C4838.
* Warnings C4065 and most C4702 (all but two).
* Warnings C4701.
* Warnings C4244.
* Warnings C4457.
* Warnings C4459.
* Some warnings C4456, due to nested `foreach` loops.

Reply

• Aug 14, 2018 - 11:14

Which is:

  • Those two remaining C4702 warnings (unreachable code), in mscore/importgtp-gp4.cpp (lines 1023 and 1082 (both understood now, but no idea how to fix)).
  • A whole bunch (407!) of (remaining) C4456 (244) and C4458 (163) warnings (variables shadowing others, either local or class members). These are relativly easy to fix, it is just a lot of work, and some guidance on preferred (re-)naming is needed.
  • Two Linker warnings LNK4221, due to empty objects for libmscore/keyfinder.obj (where all code is commented out in the corresponding cpp file with #if 0 ... #endif) and bww/writer.obj (where the corresponding cpp file doesn't contain any code at all)
  • One Linker warning LNK4098, LIBCMTD conflicting with other libs

Reply

• Aug 15, 2018 - 14:47

When building "relwithdebinfo" rather than "debug", those 2 C4702 are gone, as are those 2 Linker warning LNK4221, but 1 new warnings C4703 shows in thirdparty/poppler, 7 new warnings C4189 show in thirdparty/portmidi, the latter probably because using assert() rather than Q_ASSERT(). That warning should probably just get disabled for portmidi.

Reply

• Sep 1, 2018 - 07:01
Severity ⇒
Status (old) fixed ⇒ active
Status fixed ⇒ active

Still a bunch left:

  • Disable the 7 new warnings C4189 in thirdparty/portmidi, shown when building in "RelWithDebInfo" mode
  • Fix the 188 remaining C4456 (2 less in "RelWithDebInfo" mode)
  • Fix the 124 remaining C4458 (1 less in "RelWithDebInfo" mode)

And with the switch to 64bit builds or the change from MSVC 2015 to 2017, quite a bunch of new warnings like e.g. about int vs. size_t.

Reply

• Sep 3, 2018 - 09:28

Current status, 64bit RelWithDebInfo build:

  • C4028 Formal parameter differs from declaration, 3 times in thirdparty/portmidi
  • C4100 Unreferences formal,parameter, 1 time, code seems WIP
  • C4028 Local variable initialized but nor referenced, 7 times in thirdparty/portmidi
  • C4244 Conversion of __int64 to FT_int, possible data loss, 1 time, in thirdparty/freetype
  • C4267 Conversion of size_t to int, possibla data loss, 545 (!) times, all over the place (475 not counting those in thirdparty)
  • C4311 Shortening pointer, 9 times in thirdparty/portmidi
  • C4312 Conversion into larted type, 9 times in thirdparty/portmidi
  • C4334 32bit shift implicit changed to 64bit shift, 2 times in thirdparty/poppler
  • C4456 Shadowing of local variables, 193 times
  • C4457 Shadowing of function parameter, 2 times in read300.cpp, I guess this is WIP
  • C4458 Shadowing of class member, 122 times
  • C4701 Use of possibly uninitialized variable, 1 time.

Reply

• Sep 11, 2018 - 17:01
Severity ⇒
Status (old) fixed ⇒ active
Status fixed ⇒ active

Reopening for the remaining warnings.

Current status (with 4b5b557 in RelWithDebInfo mode) is

  • 190 (192 in Debug mode) compiler warnings C4456 (reg. a declaration shadowing a previously declared local variable, "warning C4456: declaration of 'XXX' hides previous local declaration").
  • 123 (124 in Debug mode) compiler warnings C4458 (reg. a declaration shadowing a class member, "warning C4458: declaration of 'XXX' hides class member").
  • 2 linker warnings LNK4221, due to empty objects for libmscore/keyfinder.obj (where all code is commented out in the corresponding cpp file with #if 0 ... #endif) and bww/writer.obj (where the corresponding cpp file doesn't contain any code at all), not seen on AppVeyor, but on a local build.
  • 1 linker warning LNK4098, LIBCMTD conflicting with other libs.

Reply

• Oct 12, 2018 - 05:29
Severity ⇒

Fixed in branch master, commit 474e8152cd

fix #275218: fix MSVC C4456 warnings

reg. a declaration shadowing a previously declared local
variable,
"Warning C4456: declaration of 'XXX' hides previous local
declaration"

Reply

• Oct 12, 2018 - 05:29
Severity ⇒

Fixed in branch master, commit b8810ed9e5

fix #275218: fix MSVC C4065 and C4701 warnings

ref. a `switch` without any `case` and the use of a
possibly uninitialized variable.
Also fix yet another few C4456.
Also disable one warning less for portmidi as it doesn't happen anyway
(anymore?)

Reply