I don't know history about why an earlier commit 3acc363 "Solved all compilation errors on MSVC" had to remove this #undef UNICODE. I'm on MSVC 2017 right now and I'm able to compile just fine. That commit's removal of those lines caused all the Midi device names to be identical and undescriptive as 'MMSystem,M' although the devices were perfectly valid.
it most likely got added by people using git on MS-DOS/Windows
also, add shebang to build/travis/job_macos/generateGitLog.sh
and let it keep its +x attribute, as requested by Jojo-Schmitz
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?)
* C4267 for beatroot
* C4244 and C4267 for freetype
* C4267 and C4334 for poppler
* C4028, C4189, C4267, C4311 and C4312 for portmidi
* C4267 for rtf2html
This commit contains changes to third-party code required to compile under MSVC with no warnings.
The changes in this commit are only for files in third-party code, which might be better to leave untouched for the time being.
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.
- thirdparty/diff/diff_match_patch.cpp: foreach replacement. Unreachable code.
- thirdparty/freetype/src/base/ftobjs.c: Unreachable code.
- thirdparty/freetype/src/base/ftoutln.c: Initialize vars.
- thirdparty/freetype/src/truetype/ttgload.c: Initialize vars.
- thirdparty/kQOAuth/kqoauthrequest.cpp: Unreachable code.
- thirdparty\ofqf\qoscserver.cpp: Name hiding.
- thirdparty\portmidi\pm_common\portmidi.c: Name hiding. Unreferenced parameters.
- thirdparty\portmidi\pm_win\pmwin.c: Unreachable code.
- thirdparty\portmidi\pm_win\pmwinmm.c: Unreferenced locals. Unreferenced parameters. Assignment within conditional.
- thirdparty\portmidi\porttime\porttime.c: Suppress warning C4206: "nonstandard extension used: translation unit is empty"
- thirdparty\portmidi\porttime\ptwinmm.c: Unreferenced parameters.
- thirdparty\qzip\qzip.cpp: Name hiding.
- thirdparty\rtf2html\rtf2html.cpp: Name hiding.
This commit contains changes required for MuseScore to compile under MSVC with no errors.
There are several general categories of problems that resulted in errors with the compilation. Main issues:
- Variable Length Arrays (VLA). This is a non-standard extension to C++, supported by clang toolchains, but not by MSVC. The initial workaround is to use std::vector<> instead of VLAs, eventually (if needed) with the original variable pointing to the beginning of the allocated array. More efficient alternatives are possible if profiling shows any code using VLAs to be timing-critical.
- Floating-point constants not suffixed with "f" are doubles by default; in some instances, this leads to narrowing conversion errors (in other instances, just warnings).
- MSVC does not support "or"/"and"/"not" in lieu of "||"/"&&"/"!". Changed unconditionally to use standard C++ symbols.
- MSVC does not support the "__builtin_unreachable()" compiler hint. A similar, albeit not exactly equal, alternative is "__assume(0)", which is MSVC-specific.
- MSVC does not support ranges in case statements. Replaced with list of cases instead of range (non-conditionally)
Detailed changes, with per-file comments:
- all.h: opt-in to deprecated features, include <io.h> and <process.h> instead of POSIX <unistd.h>, undefine "STRING_NONE" and "small", which Microsoft defines as macros, which result in compilation errors.
- effects/compressor/zita.cpp: eliminated narrowing conversion error by appending "f" to float constants.
- fluid/voice.cpp: appended "f" to float constants to eliminate narrowing conversion errors/warnings
- libmscore/beam.cpp: conditionally replaced VLA
- libmscore/edit.cpp: conditionally replaced VLA
- libmscore/element.cpp: appended "f" to float constant
- libmscore/fret.cpp: changed or -> ||
- libmscore/layout.cpp: conditionally replaced VLA
- libmscore/mscore.cpp: conditionally replaced "__builtin_unreachable()" with "__assume(0)" for MSVC
- libmscore/scorefile.coo: use correct char representation conversion for MSVC
- libmscore/stringdata.cpp: conditionally replaced VLA
- libmscore/system.cpp: conditionally replaced VLA
- libmscroe/text.cpp: replaced range in case statement.
- manual/genManual.cpp: use getopt() replacement.
- midi/midifile.cpp: conditionally replaced VLA. This does not use the default replacement.
- mscore/bb.cpp: replaced range in case statement.
- mscore/capella.cpp: conditionally replaced VLA. Changed and -> &&
- mscore/driver:cpp: preclude errors due to macro redefinitions.
- mscore/editstyle.cpp: conditionally replaced "__builtin_unreachable()" with "__assume(0)" for MSVC
- mscore/importgtp-gp6.cpp: conditionally replaced VLA.
- mscore/instrwidget.cpp: conditionally replaced VLA.
- mscore/jackaudio.cpp: conditionally replaced VLA. Preclude errors due to macro redefinitions.
- mscore/jackweakapi.cpp: Preclude errors due to macro redefinitions. Replacement for __atribute__((constructor)) through static object construction for MSVC. Force use of LoadLibraryA instead of LoadLibrary.
- mscore/mididriver.h: Changed not -> !
- mscore/musescore.cpp: Changed not -> !. Conditionally replaced VLA.
- mscore/resourceManager.cpp: conditionally replaced VLA.
- mscore/timeline.cpp: conditionally replaced VLA.
- omr/omrpage.cpp: conditionally replaced VLA.
- synthesizer/msynthesizer.cpp: replaced UNIX sleep(1) method with MSVC Sleep(1000) (equivalent, but in ms instead of seconds)
- synthesizer/msynthsizer.h: appended "f" to float constant
- thirdparty/poppler/config.h: set defines for MSVC to preclude the use of inexistent libraries.
- thirdparty/poppler/poppler/poppler-config.h: set defines for MSVC to preclude the use of inexistent libraries. Eliminated #defines for fmin and fmax which where causing problems.
- thirdparty/poppler/poppler/PSOutputDev.cc: added #include <algorithm> for std::min() and std::max(). Note this is required per-C++ standard.
- thirdparty/portmidi/pm_win/pmwinmm.c: undefined UNICODE to use char-based library functions.
- thirdparty/qzip/qzip.cpp: changed or -> ||
- thirdparty/rtf2html/rtf_keyword.h: file format changed from Apple to UNIX, as MSVC does not supported the former.
(NOT error related, just improvement on previous commit; manual/getopt/README.md: added link to source article)
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.
