Yoshimi/yoshimi

2.3.3: error: reference to 'set' is ambiguous

Closed this issue · 5 comments

Build fails:

In file included from /usr/ports/audio/yoshimi/work/yoshimi-2.3.3/src/CLI/CmdInterpreter.cpp:53:
/usr/ports/audio/yoshimi/work/yoshimi-2.3.3/src/Misc/Util.h:92:27: error: reference to 'set' is ambiguous
   92 | inline bool contains(std::set<T, CMP, ALO> const& set, T const& val)
      |                           ^
/usr/include/c++/v1/__tree:59:28: note: candidate found by name lookup is 'std::__1::set'
   59 | class _LIBCPP_TEMPLATE_VIS set;
      |                            ^              
/usr/ports/audio/yoshimi/work/yoshimi-2.3.3/src/Misc/Util.h:31:9: note: candidate found by name lookup is 'std::set'
   31 |   class set;
      |         ^                  
In file included from /usr/ports/audio/yoshimi/work/yoshimi-2.3.3/src/CLI/CmdInterpreter.cpp:44:

Why do you need to redefine the std::set class?

Version: 2.3.3
clang-18
FreeBSD 14.1

To explain that: actually we do not redefine std::set, rather this header only provides a forward declaration without a full definition. This is then sufficient for the compiler to parse the overloads of the contains() function.

That implies that it becomes possible to include the "Util.h" header without also forcing an include of the (possibly somewhat expensive) <set> header. But, whenever some code will require the overload specialised for std::set this code will certainly do something with a set, and thus will have the <set> include anyway, either directly or indirectly.

Generally speaking, this is a old time trick to cut down some unnecessary includes; on a large code base this can help to keep compile times down, especially with very large and monolithic headers (like GTK). The STL headers are not so much a problem, but OTOH in Yoshimi we do not use std::set so much, so I thought it might be a good idea to employ that trick. It does not seem so important in that case though, and so we may find another solution.

Remark: this kind of trickery is only necessary due to this old-time textual #include mechanism. When a code base is on C++20 and uses modules, such low-level compile tweaks become obsolete, thankfully.

This trick breaks though.

The trick as such is established practice in C / C++.

In this case however, it becomes problematic, as I seemingly made an assumption regarding the template parameters of std::set, based on my understanding of the C++ standard. And seemingly this assumption does not hold on all platforms / libraries / compilers in use.

Needless to say, we are investigating solutions to resolve this.

I must admit that I acted on assumption in this case.
I just run some time measurements and even including <set> everywhere does not have an effect above statistic fluctuation.

A closer look at dependencies reveals that in this codebase, some steps down the include graph, an assortment of even more heavyweight Standard lib headers (notably <iostream> and <functional>) is present almost everywhere.

...so we can go just with the #include and without any tricks.

This is included in our recent bugfix release v2.3.3.1