libgme / game-music-emu

Blargg's video game music emulation library, which allows audio applications to easily add playback support for the music of many classic video game consoles.
GNU Lesser General Public License v2.1
59 stars 12 forks source link

Issues compiling with GCC #83

Closed connorjclark closed 6 months ago

connorjclark commented 7 months ago

(I think this got lost in the recent move to GitHub. Originally posted here, but still stuck in "Submitted" (so never showed?))

I’m having trouble compiling GME with GCC (working great w/ clang and msvc). Here’s the output I see:

g++-13 build_gcc/_deps/gme_external-src/gme/Sap_Cpu.cpp -I build_gcc/_deps/gme_external-build/gme/ -std=c++20  -DBLARGG_BUILD_DLL -DLIBGME_VISIBILITY -DVGM_YM2612_NUKED -Dgme_EXPORTS
In file included from build_gcc/_deps/gme_external-src/gme/Classic_Emu.h:8,
                 from build_gcc/_deps/gme_external-src/gme/Sap_Emu.h:7,
                 from build_gcc/_deps/gme_external-src/gme/sap_cpu_io.h:2,
                 from build_gcc/_deps/gme_external-src/gme/Sap_Cpu.cpp:24:
build_gcc/_deps/gme_external-src/gme/Blip_Buffer.h:236:47: error: expected unqualified-id before ‘const’
  236 |         Blip_Synth<quality, range>           (const Blip_Synth<quality, range>  &) = delete;
      |                                               ^~~~~
build_gcc/_deps/gme_external-src/gme/Blip_Buffer.h:236:47: error: expected ‘)’ before ‘const’
  236 |         Blip_Synth<quality, range>           (const Blip_Synth<quality, range>  &) = delete;
      |                                              ~^~~~~
      |                                               )
build_gcc/_deps/gme_external-src/gme/Blip_Buffer.h:237:79: error: expected ‘)’ before ‘&&’ token
  237 |         Blip_Synth<quality, range>           (      Blip_Synth<quality, range> &&) = delete;
      |                                              ~                                ^~~
      |                                                                               )

Same for g++ v10 - 12. I did not test earlier versions (don’t have them on my system).

The error goes away if you delete the apparently unnecessary template parameters from the three constructor delete statements:

// disable broken defaulted constructors, Blip_Synth_ isn't safe to move/copy
Blip_Synth           (const Blip_Synth  &) = delete;
Blip_Synth           (      Blip_Synth &&) = delete;
Blip_Synth& operator=(const Blip_Synth &)  = delete;
sezero commented 6 months ago

The MinGW CI runs seem to use gcc-13 and there are no issues in there. I have no gcc-13 setup ready, so I can't test myself.

@Wohlstand? Is the proposed change correct?

sezero commented 6 months ago

I guess the problem has something to do with -std=c++20 ??

Wohlstand commented 6 months ago

The GME mainly uses C++11, or C++14 for the worst case. I have no idea why the C++20 flag is being set :thinking:

sezero commented 6 months ago

The GME mainly uses C++11, or C++14 for the worst case. I have no idea why the C++20 flag is being set 🤔

Who knows ?..

In any case, if that really is the source of the problem, then we should fix it. The solution proposed by @connorjclark works for me with my gcc-4.9.

Wohlstand commented 6 months ago

On my end is GCC-11, and the thing also works. I also have an ability to install GCC-13 separately, so, I'll try out some.

Wohlstand commented 6 months ago

On GCC 13 on my end it built just fine (Debug mode):

vitaly@whl-pc-001l:~/_git_repos_contrib/game-music-emu-orig/build-gcc-13$ cmake -DCMAKE_C_COMPILER=gcc-13 -DCMAKE_CXX_COMPILER=g++-13 -DCMAKE_BUILD_TYPE=Debug ..
-- The C compiler identification is GNU 13.1.0
-- The CXX compiler identification is GNU 13.1.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc-13 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++-13 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- VGM/GYM: Nuked OPN2 emulator will be used
-- Performing Test LINKER_SUPPORTS_NO_UNDEFINED
-- Performing Test LINKER_SUPPORTS_NO_UNDEFINED - Success
-- Performing Test LIBM_NOT_NEEDED
-- Performing Test LIBM_NOT_NEEDED - Success
-- RSN file format excluded
-- ZLib library located, compressed file formats will be supported
-- Performing Test LINKER_SUPPORTS_VERSION_SCRIPT
-- Performing Test LINKER_SUPPORTS_VERSION_SCRIPT - Success
-- SDL2 library located, player demo is available to be built in the /player directory
-- Configuring done (0.5s)
-- Generating done (0.0s)
-- Build files have been written to: /home/vitaly/_git_repos_contrib/game-music-emu-orig/build-gcc-13
vitaly@whl-pc-001l:~/_git_repos_contrib/game-music-emu-orig/build-gcc-13$ make -j 15
[  1%] Building CXX object gme/CMakeFiles/gme.dir/Blip_Buffer.cpp.o
[  5%] Building CXX object gme/CMakeFiles/gme.dir/Data_Reader.cpp.o
[  5%] Building CXX object gme/CMakeFiles/gme.dir/Classic_Emu.cpp.o
[  7%] Building CXX object gme/CMakeFiles/gme.dir/Dual_Resampler.cpp.o
[  9%] Building CXX object gme/CMakeFiles/gme.dir/Effects_Buffer.cpp.o
[ 11%] Building CXX object gme/CMakeFiles/gme.dir/Fir_Resampler.cpp.o
[ 13%] Building CXX object gme/CMakeFiles/gme.dir/gme.cpp.o
[ 15%] Building CXX object gme/CMakeFiles/gme.dir/Gme_File.cpp.o
[ 17%] Building CXX object gme/CMakeFiles/gme.dir/Multi_Buffer.cpp.o
[ 19%] Building CXX object gme/CMakeFiles/gme.dir/M3u_Playlist.cpp.o
[ 23%] Building CXX object gme/CMakeFiles/gme.dir/Ay_Apu.cpp.o
[ 23%] Building CXX object gme/CMakeFiles/gme.dir/Music_Emu.cpp.o
[ 25%] Building CXX object gme/CMakeFiles/gme.dir/Ym2612_Nuked.cpp.o
[ 27%] Building CXX object gme/CMakeFiles/gme.dir/Sms_Apu.cpp.o
[ 29%] Building CXX object gme/CMakeFiles/gme.dir/Ay_Cpu.cpp.o
[ 31%] Building CXX object gme/CMakeFiles/gme.dir/Ay_Emu.cpp.o
[ 33%] Building CXX object gme/CMakeFiles/gme.dir/Gb_Apu.cpp.o
[ 35%] Building CXX object gme/CMakeFiles/gme.dir/Gb_Cpu.cpp.o
[ 37%] Building CXX object gme/CMakeFiles/gme.dir/Gb_Oscs.cpp.o
[ 39%] Building CXX object gme/CMakeFiles/gme.dir/Gbs_Emu.cpp.o
[ 41%] Building CXX object gme/CMakeFiles/gme.dir/Gym_Emu.cpp.o
[ 43%] Building CXX object gme/CMakeFiles/gme.dir/Hes_Apu.cpp.o
[ 47%] Building CXX object gme/CMakeFiles/gme.dir/Hes_Emu.cpp.o
[ 47%] Building CXX object gme/CMakeFiles/gme.dir/Hes_Cpu.cpp.o
[ 49%] Building CXX object gme/CMakeFiles/gme.dir/Kss_Cpu.cpp.o
[ 50%] Building CXX object gme/CMakeFiles/gme.dir/Kss_Emu.cpp.o
[ 54%] Building CXX object gme/CMakeFiles/gme.dir/Kss_Scc_Apu.cpp.o
[ 54%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Apu.cpp.o
[ 56%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Cpu.cpp.o
[ 58%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Fme7_Apu.cpp.o
[ 60%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Namco_Apu.cpp.o
[ 62%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Oscs.cpp.o
[ 64%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Vrc6_Apu.cpp.o
[ 66%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Fds_Apu.cpp.o
[ 68%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Vrc7_Apu.cpp.o
[ 70%] Building C object gme/CMakeFiles/gme.dir/ext/emu2413.c.o
[ 72%] Building C object gme/CMakeFiles/gme.dir/ext/panning.c.o
[ 74%] Building CXX object gme/CMakeFiles/gme.dir/Nsf_Emu.cpp.o
[ 76%] Building CXX object gme/CMakeFiles/gme.dir/Nsfe_Emu.cpp.o
[ 78%] Building CXX object gme/CMakeFiles/gme.dir/Sap_Apu.cpp.o
[ 80%] Building CXX object gme/CMakeFiles/gme.dir/Sap_Cpu.cpp.o
[ 82%] Building CXX object gme/CMakeFiles/gme.dir/Sap_Emu.cpp.o
[ 84%] Building CXX object gme/CMakeFiles/gme.dir/Snes_Spc.cpp.o
[ 86%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Cpu.cpp.o
[ 88%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Dsp.cpp.o
[ 90%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Emu.cpp.o
[ 92%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Filter.cpp.o
[ 94%] Building CXX object gme/CMakeFiles/gme.dir/Vgm_Emu.cpp.o
[ 96%] Building CXX object gme/CMakeFiles/gme.dir/Vgm_Emu_Impl.cpp.o
[ 98%] Building CXX object gme/CMakeFiles/gme.dir/Ym2413_Emu.cpp.o
[100%] Linking CXX shared library libgme.so
[100%] Built target gme

EDIT: Release also gets built just fine.

Wohlstand commented 6 months ago

Anyway, these constructor deletions could be useful. Just in a case.

sezero commented 6 months ago

On GCC 13 on my end it built just fine (Debug mode):

EDIT: Release also gets built just fine.

Can you reproduce the issue in c++20 mode, e.g. by temporarily doing something like:

diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -54 +54 @@
-set (CMAKE_CXX_STANDARD  11)
+set (CMAKE_CXX_STANDARD  20)

Anyway, these constructor deletions could be useful. Just in a case.

Agreed

Wohlstand commented 6 months ago

Uno momento...

Wohlstand commented 6 months ago

Ye, with C++20, it explodes as issue reports: изображение

sezero commented 6 months ago

Ye, with C++20, it explodes as issue reports:

OK, then we should apply the proposed patch:

diff --git a/gme/Blip_Buffer.h b/gme/Blip_Buffer.h
index 475a375..8d0a681 100644
--- a/gme/Blip_Buffer.h
+++ b/gme/Blip_Buffer.h
@@ -235,5 +235,5 @@ public:
    // disable broken defaulted constructors, Blip_Synth_ isn't safe to move/copy
-   Blip_Synth<quality, range>           (const Blip_Synth<quality, range>  &) = delete;
-   Blip_Synth<quality, range>           (      Blip_Synth<quality, range> &&) = delete;
-   Blip_Synth<quality, range>& operator=(const Blip_Synth<quality, range> &)  = delete;
+   Blip_Synth           (const Blip_Synth  &) = delete;
+   Blip_Synth           (      Blip_Synth &&) = delete;
+   Blip_Synth& operator=(const Blip_Synth  &) = delete;
 };