This commit contains changes required for MuseScore to compile under MSVC with no warnings.
MuseScore is being compiled with the /W4 setting (warning level 4), which is similar to -wall -wextra on clang. This generates lots of warnings on MSVC, mainly for non-standard constructs and for constructs which might be bugs or might lead to bugs.
Most warnings are in the following categories:
- Name hiding: a variable hides a variable with the same name on a larger scope (or a field, or a function parameter). This can easily lead to bugs, and it is a best practice to avoid hiding variable names (see recommendation ES.12 in the C++ Core Guidelines by Stroustrop & Sutter (http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-reuse : ES.12: Do not reuse names in nested scopes)
- Narrowing conversion: a numeric conversion results in loss of significant digits (for example, double -> float). The general recommendation is to use a cast to indicate this is designed behaviour.
- Unreachable code: in several instances, there is unreachable code. The unreachable code is commented out.
- (Potentially) uninitialized local variable. Just initialized the vars.
- foreach(,) -> for(:): this does not generate a warning per-se (only a few of these generate warnings due to name hiding), but changed in keeping with "MuseScore Coding Rules" (https://musescore.org/en/handbook/musescore-coding-rules#Loops), which tells explicitly "Use C++11's "for" instead of Qt's "foreach":" ... "If you happen to be fixing some code and see a "foreach", please change that loop into a "for"."
Most changes are in the categories indicated above. The next listing shows detailed changes for files which are *not* of the aforementioned types.
- all.h: Disable warning C4127 (conditional expression is constant - generated in Qt header file qvector.h)
- awl/aslider.h: unreachable code.
- awl/knob.cpp: name hiding
- awl/mslider.cpp: name hiding
- awl/slider.cpp: name hiding
- bww2mxml/parser.cpp: name hiding
- effects/compressor/compressor.cpp: narrowing conversion
- effects/zita1/zitagui.cpp: name hiding
- fluid/fluid.cpp: foreach replacement. Name hiding.
- fluid/mod.cpp: name hiding.
- fluid/sfont.cpp: foreach replacement. Name hiding. Initialize vars.
- fluid/voice.cpp: Name hiding.
- libmscore/accidental.cpp: Name hiding.
- libmscore/ambitus.cpp: Initialize vars.
- libmscore/barline.cpp: Name hiding. Unreachable code.
- libmscore/beam.cpp: Name hiding.
- libmscore/chordrest.cpp: Unreachable code.
- libmscore/scorefile.cpp: Name hiding.
- manual/genManual.cpp: Name hiding. foreach replacement.
- midi/midifile.cpp: Name hiding. Unreachable code.
- omr/importpdf.cpp: Name hiding. foreach replacement.
- omr/omr.cpp: Name hiding. foreach replacement.
- omr/omrpage.cpp: Name hiding. foreach replacement.
- omr/omrview.cpp: Name hiding. foreach replacement.
- synthesizer/event.cpp: Unreachable code.
- zerberus\channel.cpp: Narrowing conversion.
- zerberus\instrument.cpp: Name hiding.
- zerberus\sfz.cpp: Name hiding.
- zerberus\voice.h: Suppress warning C4201: "nonstandard extension used: nameless struct/union"
- zerberus\zerberus.cpp: Name hiding. Unreferenced parameter.
- zerberus\zerberusgui.cpp: Name hiding.
In detail, changes are as follows:
- Changed .gitignore to ignore VS-specific files and directories.
- VS uses a global settings file for the CMake build process: CMakeSettings.json. This is a text file, which is conceptually equivalent to the Makefile used to invoke CMake through Make. This file might need to be changed on an individual user basis, if dependencies are not installed in default paths.
- New cmake macros to copy files in build\CopyFilesMacros.cmake. The code is from https://cmake.org/pipermail/cmake/2009-March/027892.html
- Pre-compiled headers: Visual Studio requires to create pre-compiled headers per-project (a general per-solution PCH file, although possible, is extremely problematic and not recommended). Therefore, the new macro vstudio_pch in CreatePrecompiledHeader.cmake was created to add the pre-compiled header creation step to an existing target (similar to what is done for XCode). The existing macro precompiled_header was modified to set the values for a group of variables. As part of this, the empty file all.cpp was added to the root of the source tree, as VS requires a source file to create pre-compiled headers (the header file alone can not be compiled).
- all.h is not copied to binary dir for MSVC, as I could determine no need to do this. Because of this, and the differences in PCH handling, the pseudo-targets mops1 and mops2 are not created for MSVC.
- Revised all steps conditional on toolchain, and added MSVC paths as needed. In many instances, the MSVC path is the same as the MINGW path, but not always.
- The manual (genManual) target used the getopt() functionality defined in POSIX libraries, which is not available on Windows. An LGPL'd port of getopt() for Windows was added in manual\getopt. The original source is in GitHub: nanoporetech/getopt-win32, based on a CodeProject article: https://www.codeproject.com/KB/cpp/getopt4win.aspx?msg=3987371. The corresponding CMakeLists.txt file was modified to include this files when compiling with MSVC.
- Changes in CMakeLists.txt files to create valid MSVC targets. The changes, always conditional on the MSVC toolchain, consist of:
x Setting target properties for MSVC
x Using all.h in source dir
x Adding pre-compiled headers to target
x Removing dependency from mops1 and mops2
Notes:
- The INSTALL target has NOT BEEN UPDATED for MSVC.
On Xcode, certain things don't work if the headers
aren't also part of the project, namely .h/.cpp
flipping ("Jump to Next Counterpart" in the navigation
menu) and searching the entire workspace for symbols.
The header files are now collected from the directory
of the target and show up in the XCode project under
"Header files" (per target).
I limited this change to Mac since I cannot test anywhere
else, but it would probably not hurt to do the same thing
on other platforms.
According to forums, the add_executable and add_library
commands of CMake should just silently ignore these files,
but add them to the generated target in the project.
