Fix XXH_UNREACHABLE feature test for C++23 and remove for C23 #791

[goldsteinn]

gcc11/12 don't support std::unreachable() in there -std=c++23 build.

A more reliable check is:

so use that.

A similiar issue may occur for C23 unreachable() so just remove it on the basis that if a compiler doesn't currently support some builtin for unreachable/assume its very likely not using it in analysis so we are probably not missing much (if anything) leaving it out.

[Cyan4973]

I presume you meant __STDC_VERSION__ > 202310L in this example

[goldsteinn]

Truthfully not sure. The code was based on what @t-mat suggested here.

I can drop the comment entirely if its liable to make confusion.

[t-mat]

Since C23 didn't set actual __STDC_VERSION__ yet (right?), I think we can express C23 as "beyond/greater than C17 (> 201710L)" for now.
It must be fixed after the comittee and compilers set official __STDC_VERSION__ though.
So, adding TODO or note comment would be nice to reminder.

[t-mat]

Thank you @goldsteinn for quick fix!

---

Since this PR drops support for C23, below is not a review. Just a note for (possible) future issues/improvements.

According to "7.21 Common definitions <stddef.h>" in n3054.pdf, unreachable() is a macro.
So more "safer" definition may have check for it.

#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= XXH_C23_VN)
   /* C23 and future versions have unreachable() macro */
#  include <stddef.h>
#  if defined(unreachable)
#    define XXH_UNREACHABLE() unreachable()
#  endif
#endif

If we can allow cumbersome definition, it may look like this:

/* C/C++ standard definitions */
#if !defined(XXH_UNREACHABLE)
#  if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= XXH_C23_VN)
     /* C23 and future versions have unreachable() macro */
#    include <stddef.h>      /* unreachable() */
#    if defined(unreachable)
#      define XXH_UNREACHABLE() unreachable()
#    endif
#  elif defined(__cpp_lib_unreachable) && (__cpp_lib_unreachable >= 202202L)
     /* C++23 and future versions have std::unreachable() */
#    include <utility>     /* std::unreachable() */
#    define XXH_UNREACHABLE() std::unreachable()
#  endif
#endif

/* Compiler specific definitions */
#if !defined(XXH_UNREACHABLE)
#  if XXH_HAS_BUILTIN(__builtin_unreachable)
#    define XXH_UNREACHABLE() __builtin_unreachable()
#  elif defined(_MSC_VER)
#    define XXH_UNREACHABLE() __assume(0)
#  endif
#endif

/* Fallback */
#if !defined(XXH_UNREACHABLE)
#  define XXH_UNREACHABLE()
#endif

[eseiler]

Thank you for the fix!

One more thing:
__cpp_lib_unreachable is defined in <version>.

xxhash.h currently includes

#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

None of which include <version>. So, __cpp_lib_unreachable will always be undefined. I also checked that just including xxhash.h does indeed not define the macro.

I would propose adding a

#if __has_include(<version>)
#include <version>
#endif

Before checking for __cpp_lib_unreachable.

The <version> header only exists since gcc 9, clang 9, and msvc 19.22, hence the check with __has_include.

[goldsteinn]

Thank you for the fix!

One more thing: __cpp_lib_unreachable is defined in <version>.

xxhash.h currently includes

#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

None of which include <version>. So, __cpp_lib_unreachable will always be undefined. I also checked that just including xxhash.h does indeed not define the macro.

I would propose adding a

#if __has_include(<version>)
#include <version>
#endif

Before checking for __cpp_lib_unreachable.

The <version> header only exists since gcc 9, clang 9, and msvc 19.22, hence the check with __has_include.

Hmm, so once its actually working there seems to be an issue with the extern "C" b.c <utility> contains templates.
see: https://godbolt.org/z/v773nWeYP

Not too hard to fix by closing the extern "C" before include / re-enabling after, but seems like the kind of thing that may cause an link error in the future (if std::unreachable() ever gets implemented with templates for example).
Think I'm just going to drop std::unreachable() for now.