search

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
68 stars 12 forks source link

Compilation errors in MSVC due to BLARGG_EXPORT __declspec(dllexport) #26

Closed Wohlstand closed 6 years ago

Wohlstand commented 6 years ago

Original report by jimbo1qaz (Bitbucket: jimbo1qaz, ).

On many functions, BLARGG_EXPORT (expands to __declspec(dllexport) on MSVC2017) is only present in .cpp and not .h. This fails to compile on MSVC 2017.

According to https://msdn.microsoft.com/en-us/library/a90k134d.aspx and https://gcc.gnu.org/wiki/Visibility , export statements should present in headers, not .cpp files. Many of the errors I'm getting occur when BLARGG_EXPORT is present in .cpp. I'm not sure if it should be in both or not.

I also get errors related to extern const gme_type_t \n gme_ay_type, ... where BLARGG_EXPORT is present in .cpp but not .h.

Changing the definition of BLARGG_EXPORT to "nothing" fixed the problems for my use case (static linking).

Wohlstand commented 6 years ago

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

Hmm, I agree, these export statements should be in the headers. Hopefully it's an easy fix to automate.

Wohlstand commented 6 years ago

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

Move BLARGG_EXPORT to header.

The supported C++ compilers all prefer the BLARGG_EXPORT decl to be in the declaration instead of the defintion of a symbol (plus this allows the Windows dllimport/dllexport stuff to actually work as intended)

Fixes Issue #24.

Wohlstand commented 6 years ago

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

I wasn't intending to "Resolve" this issue yet but if you could let me know if this pull request fixes the build on MSVC 2017 I'd appreciate it. If so I'll merge into master.