projectM-visualizer / projectm

projectM - Cross-platform Music Visualization Library. Open-source and Milkdrop-compatible.
https://discord.gg/mMrxAqaa3W
GNU Lesser General Public License v2.1
3.33k stars 372 forks source link

[master] Class attributes ignored in compilation? #701

Closed hartwork closed 1 year ago

hartwork commented 1 year ago

Hi!

I'm getting a compile warning with latest master that seems to point out something important:

[..]/src/libprojectM/PCM.hpp:50:23: warning: attribute ignored in declaration of ‘class Pcm’ [-Wattributes]
   50 | PROJECTM_EXPORT class Pcm
      |                       ^~~
[..]/src/libprojectM/PCM.hpp:50:23: note: attribute for ‘class Pcm’ must follow the ‘class’ keyword

And a second time for a different class:

[..]/src/libprojectM/ProjectM.hpp:69:23: warning: attribute ignored in declaration of ‘class ProjectM’ [-Wattributes]
   69 | PROJECTM_EXPORT class ProjectM
      |                       ^~~~~~~~
[..]/src/libprojectM/ProjectM.hpp:69:23: note: attribute for ‘class ProjectM’ must follow the ‘class’ keyword

If >=4.0 is supposed to only expose C API but no C++ then PROJECTM_EXPORT can just be dropped from these classes altogether? Or should just the order be fixed?

Thanks and best, Sebastian

kblaschke commented 1 year ago

It's still possible to export the C++ API via the ENABLE_CXX_INTERFACE CMake option. While it's unsupported, it is still possible for those who really need it and have no issues taking the additional workload of adapting to an ever-changing, non-stable "API".

I've already fixed this warning in my current development branch which I linked to the issue by swapping the define and class, which simply is in the wrong order.

Since it's only really useful in static libraries, this can currently be safely ignored. The warning will be gone soon as well.

kblaschke commented 1 year ago

There were some issues with the exports in some classes, these should have been addressed in the linked PR. Please test with the branch above if the error is still present, and if so, please comment again so I can fix it properly.

Sorry it took so long, but as you can see, lots of stuff was rewritten in the meantime.

hartwork commented 1 year ago

@kblaschke I can confirm that pull request #716 fixes this issue, at https://github.com/kblaschke/projectm/commit/e057d361cf6bbfa70fca29f893cc2514ed4058f4#diff-7da89f2902d45f631d48080582ab06b03edd374705919f3695800a7973e0a30bR53 precisely.

# git clone --branch rewrite-preset-parser https://github.com/kblaschke/projectm
# cd projectm
# git log -p 2>/dev/null | grep -A1 '^-PROJECTM_EXPORT class'
-PROJECTM_EXPORT class Pcm
+class PROJECTM_EXPORT PCM
--
-PROJECTM_EXPORT class ProjectM
+class PROJECTM_EXPORT ProjectM