Compile issue with gcc 11 -std=c++23

eseiler commented

2023-01-31 14:38:11 -06:00

(Migrated from github.com)

Compiling with gcc-11 -std=c++23:

xxhash.h:2052:34: error: ‘unreachable’ is not a member of ‘std’
 2052 | #  define XXH_UNREACHABLE() std::unreachable()

#782 introduced
github.com/Cyan4973/xxHash@6dba9abfaa/xxhash.h (L2049-L2052)

The problem with __cplusplus is that it does not necessarily mean that all features are available.
It's usually more robust to use the feature testing macros (__cpp_lib_unreachable).
In this case, it would mean using

#elif defined(__cpp_lib_unreachable) && (__cpp_lib_unreachable >= 202202L)

I am not sure if this also applies to the C version. GCC defines __STDC_VERSION_STDDEF_H__, but I haven't found a reference for this.

Compiling with `gcc-11 -std=c++23`: ``` xxhash.h:2052:34: error: ‘unreachable’ is not a member of ‘std’ 2052 | # define XXH_UNREACHABLE() std::unreachable() ``` #782 introduced https://github.com/Cyan4973/xxHash/blob/6dba9abfaae75a4b70a5c53d7e5eb3dd554973d1/xxhash.h#L2049-L2052 The problem with `__cplusplus` is that it does not necessarily mean that all features are available. It's usually more robust to use the [feature testing macros](https://en.cppreference.com/w/cpp/feature_test) (`__cpp_lib_unreachable`). In this case, it would mean using ```cpp #elif defined(__cpp_lib_unreachable) && (__cpp_lib_unreachable >= 202202L) ``` See also https://godbolt.org/z/5Ys77oM9j I am not sure if this also applies to the C version. [GCC](https://github.com/gcc-mirror/gcc/blob/b2ec2504af77b35e748067eeb846821d12a6b6b4/gcc/ginclude/stddef.h#L457) defines `__STDC_VERSION_STDDEF_H__`, but I haven't found a reference for this.

👍 2

Cyan4973 commented

2023-01-31 19:15:04 -06:00

(Migrated from github.com)

cc @goldsteinn

goldsteinn commented

2023-01-31 19:30:30 -06:00

(Migrated from github.com)

Compiling with gcc-11 -std=c++23:
xxhash.h:2052:34: error: ‘unreachable’ is not a member of ‘std’
 2052 | #  define XXH_UNREACHABLE() std::unreachable()
#782 introduced

github.com/Cyan4973/xxHash@6dba9abfaa/xxhash.h (L2049-L2052)

The problem with __cplusplus is that it does not necessarily mean that all features are available. It's usually more robust to use the feature testing macros (__cpp_lib_unreachable). In this case, it would mean using
#elif defined(__cpp_lib_unreachable) && (__cpp_lib_unreachable >= 202202L)
See also https://godbolt.org/z/5Ys77oM9j

Thank you, will update the CPP version to this.

I am not sure if this also applies to the C version. GCC defines __STDC_VERSION_STDDEF_H__, but I haven't found a reference for this.

By not sure if this applies to C, do you mean you're not sure if C can have the same issue if not-implement features, or you're not sure what the feature test define for something is?

My guess is if there is concern about it, we don't lose much dropping the C version. A compiler that doesn't implement some builtin for unreachable/assume is also likely to not use it an analysis even if it recognizes to stay conformant. Based on HEDLEY most of the common optimizing compilers are good with assume or __builtin_unreachable. @Cyan4973 what do you think?

> Compiling with `gcc-11 -std=c++23`: > > ``` > xxhash.h:2052:34: error: ‘unreachable’ is not a member of ‘std’ > 2052 | # define XXH_UNREACHABLE() std::unreachable() > ``` > > #782 introduced > > https://github.com/Cyan4973/xxHash/blob/6dba9abfaae75a4b70a5c53d7e5eb3dd554973d1/xxhash.h#L2049-L2052 > > The problem with `__cplusplus` is that it does not necessarily mean that all features are available. It's usually more robust to use the [feature testing macros](https://en.cppreference.com/w/cpp/feature_test) (`__cpp_lib_unreachable`). In this case, it would mean using > > ```c++ > #elif defined(__cpp_lib_unreachable) && (__cpp_lib_unreachable >= 202202L) > ``` > > See also https://godbolt.org/z/5Ys77oM9j > Thank you, will update the CPP version to this. > I am not sure if this also applies to the C version. [GCC](https://github.com/gcc-mirror/gcc/blob/b2ec2504af77b35e748067eeb846821d12a6b6b4/gcc/ginclude/stddef.h#L457) defines `__STDC_VERSION_STDDEF_H__`, but I haven't found a reference for this. By not sure if this applies to C, do you mean you're not sure if C can have the same issue if not-implement features, or you're not sure what the feature test define for something is? My guess is if there is concern about it, we don't lose much dropping the C version. A compiler that doesn't implement some builtin for unreachable/assume is also likely to not use it an analysis even if it recognizes to stay conformant. Based on [HEDLEY](https://github.com/nemequ/hedley/blob/master/hedley.h#L1228) most of the common optimizing compilers are good with `assume` or `__builtin_unreachable`. @Cyan4973 what do you think?

goldsteinn commented

2023-01-31 19:45:27 -06:00

(Migrated from github.com)

Fix is here: https://github.com/Cyan4973/xxHash/pull/792

Dropped the c23 version there as well to avoid more issues, but can obviously drop that.

Fix is here: https://github.com/Cyan4973/xxHash/pull/792 Dropped the `c23` version there as well to avoid more issues, but can obviously drop that.

👍 1

eseiler commented

2023-02-01 04:44:01 -06:00

(Migrated from github.com)

By not sure if this applies to C, do you mean you're not sure if C can have the same issue if not-implement features, or you're not sure what the feature test define for something is?

Both. I'm not sure whether C has the same issue regarding __STDC_VERSION__, and whether there is a feature test macro for it.
However, as pointed out here, unreachable was defined as a macro, so we can just check whether it's defined.

> By not sure if this applies to C, do you mean you're not sure if C can have the same issue if not-implement features, or you're not sure what the feature test define for something is? Both. I'm not sure whether C has the same issue regarding `__STDC_VERSION__`, and whether there is a feature test macro for it. However, as pointed out [here](https://github.com/Cyan4973/xxHash/pull/792#issuecomment-1411410498), `unreachable` was defined as a macro, so we can just check whether it's defined.

👍 1

goldsteinn commented

2023-02-23 17:45:01 -06:00

(Migrated from github.com)

@Cyan4973 should probably mark this resolved.

👍 1

Rows
Columns

Compile issue with gcc 11 -std=c++23 #791