From d4fbebf47896d180bc197c12284b223f1c57e2de Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Thu, 31 Mar 2022 18:50:09 +0200 Subject: [PATCH 01/10] Lower MSVC warning level to 2 (significant), enable WAE on CI --- .github/workflows/build.yml | 6 ------ CMakeLists.txt | 5 ++--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b48c493ba..19118f6a9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -125,9 +125,6 @@ jobs: export CMAKE_EXTRA_ARGS=() if [ '${{ matrix.config.compiler }}' == 'msvc' ]; then CMAKE_EXTRA_ARGS+=('-DCMAKE_GENERATOR_PLATFORM=${{ steps.windows-identify.outputs.msvc-target }}') - - # FIXME We don't want all the MSVC warnings to cause errors yet - export USE_WAE=OFF elif [ '${{ matrix.config.compiler }}' == 'mingw' ]; then CMAKE_EXTRA_ARGS+=('-DCMAKE_TOOLCHAIN_FILE=scripts/Cross-MinGW-${{ steps.windows-identify.outputs.mingw-target }}.cmake') else @@ -180,9 +177,6 @@ jobs: if [ '${{ matrix.config.compiler }}' == 'msvc' ]; then CMAKE_EXTRA_ARGS+=('-DCMAKE_GENERATOR_PLATFORM=${{ steps.windows-identify.outputs.msvc-target }}') - # FIXME We don't want all the MSVC warnings to cause errors yet - export USE_WAE=OFF - # Force static linking # 1. Make MSVC runtime configurable CMAKE_EXTRA_ARGS+=('-DCMAKE_POLICY_DEFAULT_CMP0091=NEW') diff --git a/CMakeLists.txt b/CMakeLists.txt index 769842f33..3f23e102f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -457,9 +457,8 @@ if (NOT MSVC) list(APPEND WARNING_FLAGS -Werror) endif() else() - # /wd4100 == -Wno-unused-parameter - add_compile_options("/source-charset:utf-8") - set(WARNING_FLAGS /W4 /wd4100 /D_CRT_SECURE_NO_WARNINGS) + add_compile_options("/utf-8") + set(WARNING_FLAGS /W2 /D_CRT_SECURE_NO_WARNINGS) if (WARNINGS_ARE_ERRORS) list(APPEND WARNING_FLAGS /WX) endif() From 43b12c1ca549a58c7bec9f6f5aea7b191ec0db7e Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Sat, 2 Apr 2022 18:35:00 +0200 Subject: [PATCH 02/10] Ignore C4244, C4305 & C4309 --- CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f23e102f..c240e37b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -459,6 +459,11 @@ if (NOT MSVC) else() add_compile_options("/utf-8") set(WARNING_FLAGS /W2 /D_CRT_SECURE_NO_WARNINGS) + list(APPEND WARNING_FLAGS + /wd4244 # implicit type conversions + /wd4305 # truncations + /wd4309 # truncations of constant values + ) if (WARNINGS_ARE_ERRORS) list(APPEND WARNING_FLAGS /WX) endif() From 82d893082cddae36c90bef56ad0949c9f221acad Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Thu, 31 Mar 2022 20:28:30 +0200 Subject: [PATCH 03/10] Fix for C4005: '_CRT_SECURE_NO_WARNINGS': macro redefinition --- src/engine/platform/sound/ymfm/ymfm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/platform/sound/ymfm/ymfm.h b/src/engine/platform/sound/ymfm/ymfm.h index 906e3211a..7b98f8499 100644 --- a/src/engine/platform/sound/ymfm/ymfm.h +++ b/src/engine/platform/sound/ymfm/ymfm.h @@ -33,7 +33,7 @@ #pragma once -#ifdef _MSC_VER +#if defined(_MSC_VER) && !defined(_CRT_SECURE_NO_WARNINGS) #define _CRT_SECURE_NO_WARNINGS #endif From 4c19a973efc4c57086e6d75e1ea3143d90cee36a Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Thu, 31 Mar 2022 20:46:27 +0200 Subject: [PATCH 04/10] Fix for C4805: '|': unsafe mix of type 'unsigned char' and type 'bool' in operation --- src/engine/platform/ay8930.cpp | 2 +- src/engine/platform/c64.cpp | 16 ++++++++-------- src/engine/platform/opl.cpp | 2 +- src/engine/platform/opll.cpp | 2 +- src/engine/platform/sound/ay8910.cpp | 2 +- src/gui/sysConf.cpp | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/engine/platform/ay8930.cpp b/src/engine/platform/ay8930.cpp index 1849d0cdc..eda6af041 100644 --- a/src/engine/platform/ay8930.cpp +++ b/src/engine/platform/ay8930.cpp @@ -123,7 +123,7 @@ void DivPlatformAY8930::acquire(short* bufL, short* bufR, size_t start, size_t l } while (!writes.empty()) { QueuedWrite w=writes.front(); - if (bank!=(w.addr>>4)) { + if ((int)bank!=(w.addr>>4)) { bank=w.addr>>4; ay->address_w(0x0d); ay->data_w(0xa0|(bank<<4)|ayEnvMode[0]); diff --git a/src/engine/platform/c64.cpp b/src/engine/platform/c64.cpp index 49be60fb8..6ddb2cf4e 100644 --- a/src/engine/platform/c64.cpp +++ b/src/engine/platform/c64.cpp @@ -118,7 +118,7 @@ void DivPlatformC64::acquire(short* bufL, short* bufR, size_t start, size_t len) void DivPlatformC64::updateFilter() { rWrite(0x15,filtCut&7); rWrite(0x16,filtCut>>3); - rWrite(0x17,(filtRes<<4)|(chan[2].filter<<2)|(chan[1].filter<<1)|(chan[0].filter)); + rWrite(0x17,(filtRes<<4)|(chan[2].filter<<2)|(chan[1].filter<<1)|(int)(chan[0].filter)); rWrite(0x18,(filtControl<<4)|vol); } @@ -177,7 +177,7 @@ void DivPlatformC64::tick() { } if (chan[i].std.hadWave) { chan[i].wave=chan[i].std.wave; - rWrite(i*7+4,(isMuted[i]?8:(chan[i].wave<<4))|(chan[i].ring<<2)|(chan[i].sync<<1)|chan[i].active); + rWrite(i*7+4,(isMuted[i]?8:(chan[i].wave<<4))|(chan[i].ring<<2)|(chan[i].sync<<1)|(int)(chan[i].active)); } if (chan[i].std.hadEx1) { filtControl=chan[i].std.ex1&15; @@ -232,7 +232,7 @@ int DivPlatformC64::dispatch(DivCommand c) { rWrite(c.chan*7+3,chan[c.chan].duty>>8); } if (chan[c.chan].insChanged) { - chan[c.chan].wave=(ins->c64.noiseOn<<3)|(ins->c64.pulseOn<<2)|(ins->c64.sawOn<<1)|(ins->c64.triOn); + chan[c.chan].wave=(ins->c64.noiseOn<<3)|(ins->c64.pulseOn<<2)|(ins->c64.sawOn<<1)|(int)(ins->c64.triOn); chan[c.chan].attack=ins->c64.a; chan[c.chan].decay=(ins->c64.s==15)?0:ins->c64.d; chan[c.chan].sustain=ins->c64.s; @@ -245,7 +245,7 @@ int DivPlatformC64::dispatch(DivCommand c) { if (ins->c64.initFilter) { filtCut=ins->c64.cut; filtRes=ins->c64.res; - filtControl=ins->c64.lp|(ins->c64.bp<<1)|(ins->c64.hp<<2)|(ins->c64.ch3off<<3); + filtControl=(int)(ins->c64.lp)|(ins->c64.bp<<1)|(ins->c64.hp<<2)|(ins->c64.ch3off<<3); } updateFilter(); } @@ -330,7 +330,7 @@ int DivPlatformC64::dispatch(DivCommand c) { break; case DIV_CMD_WAVE: chan[c.chan].wave=c.value; - rWrite(c.chan*7+4,(isMuted[c.chan]?8:(chan[c.chan].wave<<4))|(chan[c.chan].ring<<2)|(chan[c.chan].sync<<1)|chan[c.chan].active); + rWrite(c.chan*7+4,(isMuted[c.chan]?8:(chan[c.chan].wave<<4))|(chan[c.chan].ring<<2)|(chan[c.chan].sync<<1)|(int)(chan[c.chan].active)); break; case DIV_CMD_LEGATO: chan[c.chan].baseFreq=NOTE_FREQUENCY(c.value+((chan[c.chan].std.willArp && !chan[c.chan].std.arpMode)?(chan[c.chan].std.arp):(0))); @@ -411,11 +411,11 @@ int DivPlatformC64::dispatch(DivCommand c) { break; case 4: chan[c.chan].ring=c.value; - rWrite(c.chan*7+4,(isMuted[c.chan]?8:(chan[c.chan].wave<<4))|(chan[c.chan].ring<<2)|(chan[c.chan].sync<<1)|chan[c.chan].active); + rWrite(c.chan*7+4,(isMuted[c.chan]?8:(chan[c.chan].wave<<4))|(chan[c.chan].ring<<2)|(chan[c.chan].sync<<1)|(int)(chan[c.chan].active)); break; case 5: chan[c.chan].sync=c.value; - rWrite(c.chan*7+4,(isMuted[c.chan]?8:(chan[c.chan].wave<<4))|(chan[c.chan].ring<<2)|(chan[c.chan].sync<<1)|chan[c.chan].active); + rWrite(c.chan*7+4,(isMuted[c.chan]?8:(chan[c.chan].wave<<4))|(chan[c.chan].ring<<2)|(chan[c.chan].sync<<1)|(int)(chan[c.chan].active)); break; case 6: filtControl&=7; @@ -434,7 +434,7 @@ int DivPlatformC64::dispatch(DivCommand c) { void DivPlatformC64::muteChannel(int ch, bool mute) { isMuted[ch]=mute; - rWrite(ch*7+4,(isMuted[ch]?8:(chan[ch].wave<<4))|(chan[ch].ring<<2)|(chan[ch].sync<<1)|chan[ch].active); + rWrite(ch*7+4,(isMuted[ch]?8:(chan[ch].wave<<4))|(chan[ch].ring<<2)|(chan[ch].sync<<1)|(int)(chan[ch].active)); } void DivPlatformC64::forceIns() { diff --git a/src/engine/platform/opl.cpp b/src/engine/platform/opl.cpp index fcba9f80c..bd0ae933a 100644 --- a/src/engine/platform/opl.cpp +++ b/src/engine/platform/opl.cpp @@ -371,7 +371,7 @@ void DivPlatformOPL::tick() { if (update4OpMask) { update4OpMask=false; if (oplType==3) { - unsigned char opMask=chan[0].fourOp|(chan[2].fourOp<<1)|(chan[4].fourOp<<2)|(chan[6].fourOp<<3)|(chan[8].fourOp<<4)|(chan[10].fourOp<<5); + unsigned char opMask=(int)(chan[0].fourOp)|(chan[2].fourOp<<1)|(chan[4].fourOp<<2)|(chan[6].fourOp<<3)|(chan[8].fourOp<<4)|(chan[10].fourOp<<5); immWrite(0x104,opMask); //printf("updating opMask to %.2x\n",opMask); } diff --git a/src/engine/platform/opll.cpp b/src/engine/platform/opll.cpp index 55a1fa80c..14332bce2 100644 --- a/src/engine/platform/opll.cpp +++ b/src/engine/platform/opll.cpp @@ -620,7 +620,7 @@ int DivPlatformOPLL::dispatch(DivCommand c) { } case DIV_CMD_FM_EXTCH: if (!properDrumsSys) break; - if (properDrums==c.value) break; + if ((int)properDrums==c.value) break; if (c.value) { properDrums=true; immWrite(0x0e,0x20); diff --git a/src/engine/platform/sound/ay8910.cpp b/src/engine/platform/sound/ay8910.cpp index 2d734f931..8bb387885 100644 --- a/src/engine/platform/sound/ay8910.cpp +++ b/src/engine/platform/sound/ay8910.cpp @@ -1120,7 +1120,7 @@ void ay8910_device::sound_stream_update(short** outputs, int outLen) for (int chan = 0; chan < NUM_CHANNELS; chan++) { tone = &m_tone[chan]; - m_vol_enabled[chan] = (tone->output | tone_enable(chan)) & (noise_output() | noise_enable(chan)); + m_vol_enabled[chan] = (tone->output | (unsigned char)tone_enable(chan)) & (noise_output() | (unsigned char)noise_enable(chan)); } /* update envelope */ diff --git a/src/gui/sysConf.cpp b/src/gui/sysConf.cpp index 1bc60c7c7..8ecf7c07a 100644 --- a/src/gui/sysConf.cpp +++ b/src/gui/sysConf.cpp @@ -284,7 +284,7 @@ void FurnaceGUI::drawSysConf(int i) { } sysPal=flags&1; if (ImGui::Checkbox("PAL",&sysPal)) { - e->setSysFlags(i,(flags&(~1))|sysPal,restart); + e->setSysFlags(i,(flags&(~1))|(unsigned int)sysPal,restart); updateWindowTitle(); } bool bypassLimits=flags&4; @@ -395,4 +395,4 @@ void FurnaceGUI::drawSysConf(int i) { } break; } -} \ No newline at end of file +} From 16ad29ae7a775d5a92ff684ad5576f069ac18276 Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Thu, 31 Mar 2022 23:01:22 +0200 Subject: [PATCH 05/10] Fix C4293: '<<': shift count negative or too big, undefined behavior MSVC seems kinda braindead: https://reviews.llvm.org/D41030#952363 --- src/gui/insEdit.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/insEdit.cpp b/src/gui/insEdit.cpp index 14ca65b40..7412c15df 100644 --- a/src/gui/insEdit.cpp +++ b/src/gui/insEdit.cpp @@ -1044,7 +1044,7 @@ void FurnaceGUI::drawFMEnv(unsigned char tl, unsigned char ar, unsigned char dr, } \ ImGui::SetNextItemWidth(availableWidth); \ if (ImGui::InputText("##IMacroMML_" macroName,&mmlStr)) { \ - decodeMMLStr(mmlStr,macro,macroLen,macroLoop,macroAMin,(bitfield)?((1< Date: Sat, 9 Apr 2022 11:23:19 +0200 Subject: [PATCH 06/10] Fix C4229 Move calling convention modifier for icon callback lambda in pfd, MSVC wasn't quite happy with this. --- extern/pfd-fixed/portable-file-dialogs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extern/pfd-fixed/portable-file-dialogs.h b/extern/pfd-fixed/portable-file-dialogs.h index 008fe0a94..3d90a854f 100644 --- a/extern/pfd-fixed/portable-file-dialogs.h +++ b/extern/pfd-fixed/portable-file-dialogs.h @@ -1383,7 +1383,7 @@ inline notify::notify(std::string const &title, /* case icon::info: */ default: nid->dwInfoFlags = NIIF_INFO; break; } - ENUMRESNAMEPROC icon_enum_callback = [](HMODULE, LPCTSTR, LPTSTR lpName, LONG_PTR lParam) -> BOOL WINAPI + ENUMRESNAMEPROC icon_enum_callback = [](HMODULE, LPCTSTR, LPTSTR lpName, LONG_PTR lParam) WINAPI -> BOOL { ((NOTIFYICONDATAW *)lParam)->hIcon = ::LoadIcon(GetModuleHandle(nullptr), lpName); return false; From d5b07aa34794452543bbced4b56af8d1efc5e573 Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Sat, 9 Apr 2022 12:48:19 +0200 Subject: [PATCH 07/10] Move icon_enum_callback to separate function --- extern/pfd-fixed/portable-file-dialogs.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/extern/pfd-fixed/portable-file-dialogs.h b/extern/pfd-fixed/portable-file-dialogs.h index 3d90a854f..fc6df0a14 100644 --- a/extern/pfd-fixed/portable-file-dialogs.h +++ b/extern/pfd-fixed/portable-file-dialogs.h @@ -1331,6 +1331,14 @@ inline std::string internal::file_dialog::select_folder_vista(IFileDialog *ifd, // notify implementation +#if _WIN32 +inline BOOL WINAPI icon_enum_callback(HMODULE hModule, LPCTSTR lpType, LPTSTR lpName, LONG_PTR lParam) +{ + ((NOTIFYICONDATAW *)lParam)->hIcon = ::LoadIcon(GetModuleHandle(nullptr), lpName); + return false; +}; +#endif + inline notify::notify(std::string const &title, std::string const &message, icon _icon /* = icon::info */) @@ -1383,14 +1391,8 @@ inline notify::notify(std::string const &title, /* case icon::info: */ default: nid->dwInfoFlags = NIIF_INFO; break; } - ENUMRESNAMEPROC icon_enum_callback = [](HMODULE, LPCTSTR, LPTSTR lpName, LONG_PTR lParam) WINAPI -> BOOL - { - ((NOTIFYICONDATAW *)lParam)->hIcon = ::LoadIcon(GetModuleHandle(nullptr), lpName); - return false; - }; - nid->hIcon = ::LoadIcon(nullptr, IDI_APPLICATION); - ::EnumResourceNames(nullptr, RT_GROUP_ICON, icon_enum_callback, (LONG_PTR)nid.get()); + ::EnumResourceNames(nullptr, RT_GROUP_ICON, (ENUMRESNAMEPROC)icon_enum_callback, (LONG_PTR)nid.get()); nid->uTimeout = 5000; From ae4968318e978709e77e6ebf5d834f9df492d807 Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Sat, 9 Apr 2022 14:33:13 +0200 Subject: [PATCH 08/10] Tell SDL headers in MSVC build that we have a libc --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c240e37b6..fef510efb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -177,6 +177,10 @@ else() add_subdirectory(extern/SDL EXCLUDE_FROM_ALL) list(APPEND DEPENDENCIES_INCLUDE_DIRS extern/SDL/include) list(APPEND DEPENDENCIES_LIBRARIES SDL2-static) + # Work around add_subdirectory'd SDL not propagating HAVE_LIBC to MSVC furnace build + if (MSVC) + list(APPEND DEPENDENCIES_COMPILE_OPTIONS "/DHAVE_LIBC") + endif() message(STATUS "Using vendored SDL2") endif() From b6fb3820da16d5ea05204b00dda347025f209f4b Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Sat, 9 Apr 2022 21:00:38 +0200 Subject: [PATCH 09/10] Remove unneeded includes Some are harmless, some screw with SDL's standard includes due to preceeding `_USE_MATH_DEFINES`. --- src/engine/engine.cpp | 3 --- src/engine/filter.cpp | 3 +-- src/engine/playback.cpp | 3 --- src/gui/pattern.cpp | 2 -- 4 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/engine/engine.cpp b/src/engine/engine.cpp index 78ae1ce49..a154277e8 100644 --- a/src/engine/engine.cpp +++ b/src/engine/engine.cpp @@ -17,9 +17,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "dataErrors.h" -#include "song.h" -#include #define _USE_MATH_DEFINES #include "engine.h" #include "instrument.h" diff --git a/src/engine/filter.cpp b/src/engine/filter.cpp index 729c8caa1..8e0ac02eb 100644 --- a/src/engine/filter.cpp +++ b/src/engine/filter.cpp @@ -17,7 +17,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include #define _USE_MATH_DEFINES #include #include "filter.h" @@ -85,4 +84,4 @@ float* DivFilterTables::getSincIntegralTable() { } } return sincIntegralTable; -} \ No newline at end of file +} diff --git a/src/engine/playback.cpp b/src/engine/playback.cpp index 8e1f1d664..409b4a08a 100644 --- a/src/engine/playback.cpp +++ b/src/engine/playback.cpp @@ -17,9 +17,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "blip_buf.h" -#include "song.h" -#include "wavetable.h" #define _USE_MATH_DEFINES #include "dispatch.h" #include "engine.h" diff --git a/src/gui/pattern.cpp b/src/gui/pattern.cpp index d4bf904b4..d6b6fce70 100644 --- a/src/gui/pattern.cpp +++ b/src/gui/pattern.cpp @@ -17,8 +17,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include -#include #define _USE_MATH_DEFINES #include "gui.h" #include "../ta-log.h" From 366a7a24a009bca878e768841f713a012c34821b Mon Sep 17 00:00:00 2001 From: OPNA2608 Date: Sat, 9 Apr 2022 21:22:25 +0200 Subject: [PATCH 10/10] Decide amount of build cores on CI based on platform GitHub-hosted macOS runners actually get 3 cores instead of 2, according to https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources --- .github/workflows/build.yml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 19118f6a9..2fbae0f51 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -89,6 +89,19 @@ jobs: echo "::set-output name=id::${package_name}" echo "::set-output name=filename::${package_name}${package_ext}" + - name: Set build cores amount + id: build-cores + run: | + # https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources + set amount=2 + if [ '${{ runner.os }}' == 'macOS' ]; then + amount=3 + fi + + echo "Amount of cores we can build with: ${amount}" + + echo "::set-output name=amount::${amount}" + - name: Setup Toolchain [Windows MSVC] if: ${{ matrix.config.compiler == 'msvc' }} uses: seanmiddleditch/gha-setup-vsdevenv@v3 @@ -156,7 +169,7 @@ jobs: cmake \ --build ${PWD}/build \ --config ${{ env.BUILD_TYPE }} \ - --parallel 2 + --parallel ${{ steps.build-cores.outputs.amount }} - name: Install (System Libraries) if: ${{ runner.os == 'Linux' && matrix.config.compiler != 'mingw' }} @@ -205,7 +218,7 @@ jobs: cmake \ --build ${PWD}/build \ --config ${{ env.BUILD_TYPE }} \ - --parallel 2 + --parallel ${{ steps.build-cores.outputs.amount }} - name: Package [Windows] if: ${{ runner.os == 'Windows' || matrix.config.compiler == 'mingw' }}