Issue 41617: __builtin_bswap16 is used without checking it is supported

Older clang versions don't have __builtin_bswap16, but it's always used when compiling with clang, which means the build fails with those older versions. The code should use __has_builtin to check.

> Older clang versions don't have __builtin_bswap16,

According to https://github.com/nodejs/node/pull/7644 it's available in clang 3.2 but not in clang 3.0.

I wrote PR 21949 to fix the issue: it only uses __has_builtin() if __clang__ is defined, it's different than PR 21942.

Joshua Root: I'm curious. Can I ask you on which platform do you use clang older than 3.2? It has been released 8 years ago, December 2012. The LLVM project is known to evolve very quickly!

New changeset e6905e4c82cc05897dc1bf5ab2b5b94b2b043a7f by Victor Stinner in branch 'master':
bpo-41617: Fix pycore_bitutils.h to support clang 3.0 (GH-22042)
https://github.com/python/cpython/commit/e6905e4c82cc05897dc1bf5ab2b5b94b2b043a7f

New changeset 4217b3c12809b070928413f75949a7ddc4f2221c by Victor Stinner in branch '3.9':
bpo-41617: Fix pycore_byteswap.h to support clang 3.0 (GH-22042) (GH-22044)
https://github.com/python/cpython/commit/4217b3c12809b070928413f75949a7ddc4f2221c

Thank you Joshua Root for your bug report! Your PR 21942 was good, but I chose to avoid __has_builtin() since it is not supported by all compilers and only available since GCC 10 for GCC.

> I'm curious. Can I ask you on which platform do you use clang older than 3.2?

Mac OS X 10.7 / Xcode 4.6.3. I'm not using it personally, but we have automated builds on that platform.

Unfortunately the patch ultimately committed did not fix the build there. Clang reports its version as "Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn)". __clang_major__ is 4 and __clang_minor__ is 2.

Apple's versioning scheme is different to that of LLVM upstream, which is one reason why I preferred detecting features directly rather than inserting externally-derived knowledge about which versions provide which features.

Apologies for not getting back to you about this sooner; the notifications appear to have gotten lost.

> Unfortunately the patch ultimately committed did not fix the build there. Clang reports its version as "Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn)". __clang_major__ is 4 and __clang_minor__ is 2.

Oh... I wrote PR 23260 which is based on your original PR 21942. I added a new _Py__has_builtin() macro rather than defining a __has_builtin() which always returns 0.

New changeset b3b98082c5431e77c64cab2c85525a804436b505 by Victor Stinner in branch 'master':
bpo-41617: Add _Py__has_builtin() macro (GH-23260)
https://github.com/python/cpython/commit/b3b98082c5431e77c64cab2c85525a804436b505

New changeset ec306a2fd91d8b961b2a80c080dd2262bb17d862 by Victor Stinner in branch '3.9':
bpo-41617: Add _Py__has_builtin() macro (GH-23260) (GH-23262)
https://github.com/python/cpython/commit/ec306a2fd91d8b961b2a80c080dd2262bb17d862

Ok, I hope that this issue it's really fixed ;-) Thanks again Joshua Root.

Confirmed fixed in 3.9.1 and 3.10.0a3. Thanks Victor!

> Confirmed fixed in 3.9.1 and 3.10.0a3. Thanks Victor!

Woooot!

messages: + msg380902
stage: patch review -> resolved

messages: + msg377149