libgig future vs error=overloaded-virtual and other problems

[FyiurAmron]

System Information

GCC >= 13.0

LMMS Version(s)

1.3.0-alpha (master)

Most Recent Working Version

none AFAIK

Bug Summary

Similarly to #7284 , libgig breaks on g++ >= 13 due to overloaded virtuals combo. See https://github.com/LMMS/lmms/actions/runs/9294048041/job/25578429009#step:9:2766 for concrete example. However, libgig is much more tricky to handle here:

1. libgig-4.4.1 (ATM newest) doesn't have a fix for the problem,
2. we are basing our MinGW builds on outdated version from tobydox repo (4.0.0) and, to make things worse, MSYS2 doesn't have this lib built at all,
3. disabling the warning is trickier here as well vs veal, as we don't own the code in any way ATM,
4. there is no up-to-date repo for libgig in git/GitHub ecosystem (there's an SVN repo, though)

I've contacted Christian Schoenebeck about the possible upstream fix, however this still begs the question how we should handle this case, as it:

• currently blocks update to GCC 13,
• there is no guarantee of upstream fix at all (although TBH it's quite likely),
• we still wouldn't be able to consume libgig then without including building/packaging the dep somehow.

#7162 is the 2nd part of this problem. Related is e.g. #7162 (comment) .

CC @Rossmaxx @Veratil

Expected Behaviour

Build on g++ >= 13.0 should work.

Steps To Reproduce

1. get gcc/g++ >= 13.0
2. try to build
3. notice the warnings/errors

Logs

Click to expand

In file included from /usr/x86_64-w64-mingw32/include/libgig/gig.h:27,
                 from /__w/lmms/lmms/plugins/GigPlayer/GigPlayer.h:41,
                 from /__w/lmms/lmms/plugins/GigPlayer/GigPlayer.cpp:31:
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:354:26: error: 'virtual void DLS::Resource::CopyAssign(const DLS::Resource*)' was hidden [-Werror=overloaded-virtual=]
  354 |             virtual void CopyAssign(const Resource* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:414:27: note:   by 'virtual void DLS::Sample::CopyAssign(const DLS::Sample*)'
  414 |             virtual void  CopyAssign(const Sample* orig);
      |                           ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:354:26: error: 'virtual void DLS::Resource::CopyAssign(const DLS::Resource*)' was hidden [-Werror=overloaded-virtual=]
  354 |             virtual void CopyAssign(const Resource* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:445:26: note:   by 'virtual void DLS::Region::CopyAssign(const DLS::Region*)'
  445 |             virtual void CopyAssign(const Region* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:293:27: error: 'virtual void DLS::Articulator::CopyAssign(const DLS::Articulator*)' was hidden [-Werror=overloaded-virtual=]
  293 |             virtual void  CopyAssign(const Articulator* orig);
      |                           ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:445:26: note:   by 'virtual void DLS::Region::CopyAssign(const DLS::Region*)'
  445 |             virtual void CopyAssign(const Region* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:378:26: error: 'virtual void DLS::Sampler::CopyAssign(const DLS::Sampler*)' was hidden [-Werror=overloaded-virtual=]
  378 |             virtual void CopyAssign(const Sampler* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:445:26: note:   by 'virtual void DLS::Region::CopyAssign(const DLS::Region*)'
  445 |             virtual void CopyAssign(const Region* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:354:26: error: 'virtual void DLS::Resource::CopyAssign(const DLS::Resource*)' was hidden [-Werror=overloaded-virtual=]
  354 |             virtual void CopyAssign(const Resource* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:473:26: note:   by 'virtual void DLS::Instrument::CopyAssign(const DLS::Instrument*)'
  473 |             virtual void CopyAssign(const Instrument* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:293:27: error: 'virtual void DLS::Articulator::CopyAssign(const DLS::Articulator*)' was hidden [-Werror=overloaded-virtual=]
  293 |             virtual void  CopyAssign(const Articulator* orig);
      |                           ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:473:26: note:   by 'virtual void DLS::Instrument::CopyAssign(const DLS::Instrument*)'
  473 |             virtual void CopyAssign(const Instrument* orig);
      |                          ^~~~~~~~~~
/usr/x86_64-w64-mingw32/include/libgig/DLS.h:473:26: error: 'virtual void DLS::Instrument::CopyAssign(const DLS::Instrument*)' was hidden [-Werror=overloaded-virtual=]
/usr/x86_64-w64-mingw32/include/libgig/gig.h:1125:18: note:   by 'gig::Instrument::CopyAssign'
 1125 |             void CopyAssign(const Instrument* orig, const std::map* mSamples);
      |                  ^~~~~~~~~~

Screenshots / Minimum Reproducible Project

build job

Please search the issue tracker for existing bug reports before submitting your own.

• I have searched all existing issues and confirmed that this is not a duplicate.

[DomClark]

I have a WIP branch to avoid applying -Wall to third-party code, but had to leave it be as it needed newer CMake features. Now we have a much newer version available, I should be able to pick it up again next week.

[FyiurAmron]

@DomClark seems like a very good idea in general and also a good starting point here, as it's likely we'd stumble upon this kind of problems in the future again, and obviously not all of our current deps are maintained good enough for the contribs to arrive in reasonable time.

[Rossmaxx]

Since you said you contacted Christian, I am pretty sure we'll get a 4.4.2 release soon. Just keep a look at the mailing list. Once that's done, I'll handle updating the vcpkg port. He has been quite cooperative for updates when I sent him patches when i worked with MSVC support for #7162 .

For you to create patches, you can download the tarball, initiate a git repo there, create commit and do git diff <previous commit> <head commit> > fix.patch (not sure about the order of commits tho) and then put it in the mailing this.

[FyiurAmron]

@Rossmaxx et al.

From: Christian Schoenebeck [email protected]

Hi Marcin,

first off, in case you want to continue this discussion, then please move it
to the linuxsampler-devel mailing list.

I'm writing to you about possible fix to the aforementioned
problem. As per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87729 , g++
versions >= 13.0 have -Woverloaded-virtual in -Wall now (as visible in
https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED&resolution=FIXE
D&target_milestone=13.0 ). This, however, comes with caveats (
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109740 ) - and, in this case,
breaks any file including e.g. DLS.h or gig.h (possibly also others) - see
e.g.

AFAICS the problem is not -Wall, because that just emits warnings. The problem
rather appears that you additionally built LMMS with -Werror which tells GCC
to handle all warnings as errors, which is quite aggressive, and not GCC's
fault.

https://github.com/LMMS/lmms/actions/runs/9294048041/job/25578429009#step:9:
2766 . The fix is relatively easy on the lib side, as it's basically just
adding a couple using xyz::baseVirtualMethod in the child class, like
e.g.
LMMS/veal@a64edc8
22541a4aa3#diff-486bc0aa508c942c5fd7b06e89c0d31ac1de45b077bde0584d13d1f78982
5362 . Would it be possible to get a new libgig version that would have this
problem patched?

So in libgig's specific case -Woverloaded-virtual warns about a potential
build error if a library user would call that method with a wrong argument.
How much sense does that make, please?

I already find this warning pointless, but even promoting that warning to an
error is completely nonsense.

I would recommend that you just add -Wno-error=overloaded-virtual on your end.

#bikeshedding

tl;dr he doesn't want to change it. Oh well, Another Day in Internet. :D
_{edited at the request of the offended parties}

@DomClark
seems your idea is the best one here ATM then :D

[DomClark]

I have a WIP branch to avoid applying -Wall to third-party code

PR open at #7319.

[FyiurAmron]

yup, this as good of a solution as we can easily have ATM, #completed :)