g++/gcc-8 compatibility tweak for <bit> library header

[pcdion]

We are using gcc-8 for CI/CD, which supports C++20 features. While troubleshooting a build error, I came across the following:

Note: std::endian began in <type_traits>, but it [was moved](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1612r1.pdf) to <bit> at the 2019 Cologne meeting. GCC 8, Clang 7, 8 and 9 have it in <type_traits> while GCC 9+ and Clang 10+ have it in <bit>. -- https://stackoverflow.com/questions/1001307/detecting-endianness-programmatically-in-a-c-program

In decoder_abstract.cc and bitstream.cc, this tweak can overcome the issue:

#if defined(GNUC) && GNUC < 9
#include <type_traits>
#else
#include
#endif

Would it be possible to include something like that?

Thanks

@farindk

[bradh]

This is really working around a bug in GCC 8 right? (in that it pre-empted the final decision). Any reason not just to upgrade the CI/CD pipeline to GCC 9+ or Clang 10+ ?

[farindk]

Changed in above commit. That doesn't seem work for Clang, but until nobody needs that, it'll be ok.

[farindk]

This is really working around a bug in GCC 8 right? (in that it pre-empted the final decision). Any reason not just to upgrade the CI/CD pipeline to GCC 9+ or Clang 10+ ?

I have no problems with some #ifdefs for legacy compiler support. Later, we can take it out again.

[jerstlouis]

Thaks for the fix @farindk !

That doesn't seem work for Clang

In the description above @pcdion forgot to quote the code, so GNUC was intended to be __GNUC__, sorry.
The following should work, including for older and newer Clang:

#if (defined(__GNUC__) && __GNUC__ < 9) || (defined(__clang__) && __clang_major__ < 10)
#include <type_traits>
#else
#include <bit>
#endif

@bradh

This is really working around a bug in GCC 8 right? (in that it pre-empted the final decision).

I would call it a limitation rather than a bug (GCC 8 is otherwise capable of building libheif). C++ 20 support in GCC is still marked as experimental even in the very latest compiler.

From https://gcc.gnu.org/projects/cxx-status.html#cxx20 :

Important: Because the ISO C++20 standard is very recent, GCC's support is experimental.

Even for C++ 17:

GCC has almost full support for the 2017 revision of the C++ standard.

Any reason not just to upgrade the CI/CD pipeline to GCC 9+ or Clang 10+ ?

The main reason is that it would take a lot of time which we are not ready to invest right now (several inter-dependent projects). Building on an older system also maximizes compatibility to deploy on both older and newer systems. We unfortunately already had to spend a lot of efforts to upgrade to GCC 8 (for Windows / MinGW-w64 builds in particular).

I find it ironic that C++ makes you rely on cutting-edge standard and experimental compiler features to detect the relic of the past that is big-endianness :)

[farindk]

I changed the #if to include Clang.

I find it ironic that C++ makes you rely on cutting-edge standard and experimental compiler features to detect the relic of the past that is big-endianness :)

Better late than never.

[kmilos]

As Clang (and Intel, PGI and maybe others) also define __GNUC__, this can potentially catch all Clang versions (value is 4 even w/ my Clang > 10), which is not what one wants. So perhaps check for __clang__ first, and then for __GNUC__? Or use specific GCC version macros, not just __GNUC__?

From https://sourceforge.net/p/predef/wiki/Compilers/

Notice that the meaning of the __GNUC__ macro has changed subtly over the years, from identifying the GNU C/C++ compiler to identifying any compiler that implements the GNU compiler extensions

[jerstlouis]

Thanks @kmilos for pointing this out... (GNU extensions vs. GCC compiler).

I guess something like this would be needed:

#define GCC_COMPILER (defined(__GNUC__) && !defined(__clang__) && !defined(__INTEL_COMPILER) && !defined(__PGI))
#if (defined(GCC_COMPILER) && __GNUC__ < 9) || (defined(__clang__) && __clang_major__ < 10)
#include <type_traits>
#else
#include <bit>
#endif

See also https://stackoverflow.com/questions/28166565/detect-gcc-as-opposed-to-msvc-clang-with-macro .

[farindk]

That's probably longer than the std::endian implementation :-)

[jerstlouis]

#define IS_BIG_ENDIAN (!*(unsigned char *)&(uint16_t){1})

(from https://stackoverflow.com/questions/2100331/macro-definition-to-determine-big-endian-or-little-endian-machine)

which also says:

In general though, you should try to write code that does not depend on the endianness of the host platform.

Sorry to have opened this can of worms ;) I blame this moving of a feature implemented in multiple major compiler releases to a different header though.

[farindk]

Thanks

[bradh]

To further complicate it, I think this has broken the AppVeyor build:

https://ci.appveyor.com/project/strukturag/libheif/builds/51041650/job/8g8ocxxntdd4b5vp

I think we need <bit> for the std::bit_cast<> implementation.

[jerstlouis]

Thanks @bradh and sorry again for all the troubles.
Isn't this a failing MSVC build? Is this defining__GNUC__ or __clang__ somehow?

Otherwise <bit> should still be included exactly as before.

Or perhaps this is a case of an older MSVC compiler that needs a similar compatibility fix?

[farindk]

The above fixes the compiler version detection.

The second line #if defined(GCC_COMPILER) is wrong, because GCC_COMPILER is always defined in the line above.
Removing the defined() also does not work, because expanding the macro from the first line, including a defined is undefined behavior.