Re: byteswap: Use inline functions instead of macros.

[Paul Eggert]

+#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.


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.

Re: byteswap: Use inline functions instead of macros.

[Collin Funk]

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?

CollinFrom 73541d459cdd964e71f40ad447e50a7253a6f20d Mon Sep 17 00:00:00 2001
From: Collin Funk 
Date: Thu, 16 May 2024 23:18:16 -0700
Subject: [PATCH] byteswap: Use __has_builtin portably.

Reported by Paul Eggert in
.

* 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  
+
+	byteswap: Use __has_builtin portably.
+	Reported by Paul Eggert in
+	.
+	* 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  
 
 	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) & 0xff00u) >> 24) | (((x) & 0x00ffu) >> 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) & 0xff00ull) >> 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

Re: byteswap: Use inline functions instead of macros.

[Bruno Haible]

Collin Funk asked:
> I'm assuming that _GL_[module-name]_(rest-of-macro) can be assumed
> safe?

Yes, such a macro name is safe.

Bruno

Re: byteswap: Use inline functions instead of macros.

[Paul Eggert]

On 2024-05-16 23:25, Collin Funk wrote:

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?


No, it was expected behavior before __has_builtin was standardized, the 
same behavior you'd get now with "#if defined FOO && FOO (x)" when FOO 
is not defined.