Hi Paul,
On 5/16/24 10:16 PM, Paul Eggert wrote:
>> +#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) \
>> + || (defined __has_builtin && __has_builtin (__builtin_bswap32))
>
> Unfortunately this usage is not portable, not for __has_builtin or for any of
> the other __has_XX primitives. This is because older compilers will replace
> "defined __has_builtin && __has_builtin (__builtin_bswap32)" with "1 && 0
> (__builtin_bswap32)" and then complain about the bad syntax.
Oops, I wasn't aware of this. Was that a bug in old versions?
> You can safely use __has_builtin (X) only inside a block that is protected by
> checking that __has_builtin is defined. See stdbit.in.h for an example.
I've applied the attached patch. It should be pretty the same method
as stdbit.in.h.
I'm assuming that _GL_[module-name]_(rest-of-macro) can be assumed
safe?
Collin
From 73541d459cdd964e71f40ad447e50a7253a6f20d Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Thu, 16 May 2024 23:18:16 -0700
Subject: [PATCH] byteswap: Use __has_builtin portably.
Reported by Paul Eggert in
<https://lists.gnu.org/archive/html/bug-gnulib/2024-05/msg00249.html>.
* lib/byteswap.in.h (_GL_BYTESWAP_HAS_BUILTIN_BSWAP16)
(_GL_BYTESWAP_HAS_BUILTIN_BSWAP32)
(_GL_BYTESWAP_HAS_BUILTIN_BSWAP64): Define using the GCC version or
__has_builtin after checking that it is defined.
(bswap_16, bswap_32, bswap_64): Use the macros.
* modules/byteswap (Depends-on): Add stdbool as a conditional
dependency.
---
ChangeLog | 13 +++++++++++++
lib/byteswap.in.h | 29 +++++++++++++++++++++++------
modules/byteswap | 1 +
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 5238067cf9..742a20d012 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2024-05-16 Collin Funk <collin.fu...@gmail.com>
+
+ byteswap: Use __has_builtin portably.
+ Reported by Paul Eggert in
+ <https://lists.gnu.org/archive/html/bug-gnulib/2024-05/msg00249.html>.
+ * lib/byteswap.in.h (_GL_BYTESWAP_HAS_BUILTIN_BSWAP16)
+ (_GL_BYTESWAP_HAS_BUILTIN_BSWAP32)
+ (_GL_BYTESWAP_HAS_BUILTIN_BSWAP64): Define using the GCC version or
+ __has_builtin after checking that it is defined.
+ (bswap_16, bswap_32, bswap_64): Use the macros.
+ * modules/byteswap (Depends-on): Add stdbool as a conditional
+ dependency.
+
2024-05-16 Paul Eggert <egg...@cs.ucla.edu>
putenv-tests: pacify gcc -Wdiscarded-qualifiers
diff --git a/lib/byteswap.in.h b/lib/byteswap.in.h
index cfa3f04031..ae05d59441 100644
--- a/lib/byteswap.in.h
+++ b/lib/byteswap.in.h
@@ -34,13 +34,32 @@ _GL_INLINE_HEADER_BEGIN
extern "C" {
#endif
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
+# define _GL_BYTESWAP_HAS_BUILTIN_BSWAP16 true
+#elif defined __has_builtin
+# if __has_builtin (__builtin_bswap16)
+# define _GL_BYTESWAP_HAS_BUILTIN_BSWAP16 true
+# endif
+#endif
+
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
+# define _GL_BYTESWAP_HAS_BUILTIN_BSWAP32 true
+# define _GL_BYTESWAP_HAS_BUILTIN_BSWAP64 true
+#elif defined __has_builtin
+# if __has_builtin (__builtin_bswap32)
+# define _GL_BYTESWAP_HAS_BUILTIN_BSWAP32 true
+# endif
+# if __has_builtin (__builtin_bswap64)
+# define _GL_BYTESWAP_HAS_BUILTIN_BSWAP64 true
+# endif
+#endif
+
/* Given an unsigned 16-bit argument X, return the value corresponding to
X with reversed byte order. */
_GL_BYTESWAP_INLINE uint16_t
bswap_16 (uint16_t x)
{
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) \
- || (defined __has_builtin && __has_builtin (__builtin_bswap16))
+#ifdef _GL_BYTESWAP_HAS_BUILTIN_BSWAP16
return __builtin_bswap16 (x);
#else
return (((((x) >> 8) & 0xff) | (((x) & 0xff) << 8)));
@@ -52,8 +71,7 @@ bswap_16 (uint16_t x)
_GL_BYTESWAP_INLINE uint32_t
bswap_32 (uint32_t x)
{
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) \
- || (defined __has_builtin && __has_builtin (__builtin_bswap32))
+#ifdef _GL_BYTESWAP_HAS_BUILTIN_BSWAP32
return __builtin_bswap32 (x);
#else
return ((((x) & 0xff000000u) >> 24) | (((x) & 0x00ff0000u) >> 8)
@@ -66,8 +84,7 @@ bswap_32 (uint32_t x)
_GL_BYTESWAP_INLINE uint64_t
bswap_64 (uint64_t x)
{
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) \
- || (defined __has_builtin && __has_builtin (__builtin_bswap64))
+#ifdef _GL_BYTESWAP_HAS_BUILTIN_BSWAP64
return __builtin_bswap64 (x);
#else
return ((((x) & 0xff00000000000000ull) >> 56)
diff --git a/modules/byteswap b/modules/byteswap
index 82fe424a61..6e777a699b 100644
--- a/modules/byteswap
+++ b/modules/byteswap
@@ -9,6 +9,7 @@ m4/byteswap.m4
Depends-on:
gen-header
extern-inline [$GL_GENERATE_BYTESWAP_H]
+stdbool [$GL_GENERATE_BYTESWAP_H]
stdint [$GL_GENERATE_BYTESWAP_H]
configure.ac:
--
2.45.0
