thliebig / openEMS

openEMS is a free and open-source electromagnetic field solver using the EC-FDTD method.
http://openEMS.de
GNU General Public License v3.0
413 stars 146 forks source link

Set C++ version for boost thread #137

Closed aWZHY0yQH81uOYvH closed 5 months ago

aWZHY0yQH81uOYvH commented 5 months ago

Fixes https://github.com/thliebig/openEMS-Project/issues/190

As of recent, nf2ff has been failing to compile with the latest version of boost (1.84.0):

In file included from /tmp/openEMS-Project/openEMS/nf2ff/nf2ff.cpp:19:
In file included from /tmp/openEMS-Project/openEMS/nf2ff/nf2ff_calc.h:26:
In file included from /usr/local/include/boost/thread.hpp:24:
In file included from /usr/local/include/boost/thread/future.hpp:31:
In file included from /usr/local/include/boost/thread/detail/invoker.hpp:32:
/usr/local/include/boost/thread/csbl/tuple.hpp:19:18: error: no member named 'tuple' in namespace 'std'
    using ::std::tuple;
          ~~~~~~~^
/usr/local/include/boost/thread/csbl/tuple.hpp:21:18: error: no member named 'make_tuple' in namespace 'std'
    using ::std::make_tuple;
          ~~~~~~~^

They changed around some stuff in this header file.

Setting the C++ version to 11 fixes this for me on

% clang --version
Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: x86_64-apple-darwin23.3.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
thliebig commented 5 months ago

Hmm, I'm always a but hesitant with such changes as people may want to compile openEMS on e.g. very old machines with potentially an even older compiler?
And on my quite up to date gcc I do not see this issue either. Maybe we should put this behind a compiler switch (only for clang) or only for mac?? At least for the time being? Opinions?

aWZHY0yQH81uOYvH commented 5 months ago

On my install of GCC 13, it looks like the default __cplusplus version without any --std argument supplied is C++ 17, whereas on clang it's C++ 98. Specifying --std=c++98 for GCC makes it spit out even more errors than clang when dealing with the same boost/thread.hpp header.

It's mainly a problem with the combination of the old C++ standard and the new version of Boost, which uses C++ 11 features. Presumably an old machine that doesn't have a C++ 11 compiler wouldn't also be using a new version of Boost that requires a C++ 11 compiler.

I suppose the inclusion of this CMake line could be made dependent on the detected Boost version rather than the host platform or compiler? But honestly, I doubt there are many machines out there worth running openEMS on that don't have a C++ 11 compiler.

aWZHY0yQH81uOYvH commented 5 months ago

Another consideration would be if this change (whatever its final form may be) should be moved elsewhere in the project, since nf2ff isn't the only thing using Boost. This specific header file is also used in the openEMS multithread engine as well.

aWZHY0yQH81uOYvH commented 5 months ago

It looks like setting the C++ version to something the current compiler doesn't support is OK. According to the CMake docs,

If [CXX_STANDARD_REQUIRED] is OFF or unset, the CXX_STANDARD target property is treated as optional and may "decay" to a previous standard if the requested is not available.

So this shouldn't break the compilation with an older compiler as long as whatever dependency libraries are OK with the older C++ standard.

thliebig commented 5 months ago

Thanks for the analysis. Let's just apply this and cross our fingers that nothing breaks :wink:

aWZHY0yQH81uOYvH commented 5 months ago

Sounds good 🙂

Looks like something somewhere is setting C++ 11 for the main openEMS build, which is why this change is only needed for nf2ff, so It's Probably Fine™