bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* Keep config.h idempotent
@ 2023-01-25 14:05 Bruno Haible
  2023-01-25 22:00 ` Paul Eggert
  0 siblings, 1 reply; 4+ messages in thread
From: Bruno Haible @ 2023-01-25 14:05 UTC (permalink / raw)
  To: bug-gnulib, epsilon-devel; +Cc: poke-devel

Compiling GNU poke 2.90.1 on the gcc401.fsffrance.org machine, I get a
compilation failure:

gcc -DHAVE_CONFIG_H -I. -I../../jitter -I./config-private -I./jitter  -I./gnulib-local -I../../jitter/gnulib-local -DJITTER_WITH_LIBTEXTSTYLE=1  -DJITTER_FLAGDIR=\"/home/haible/poke-2.90.1/build/jitter/flags\" -DJITTER_TEMPLATEDIR=\"/home/haible/poke-2.90.1/build/jitter/../../jitter/templates\" -DJITTER_INTERNAL=1 -I/home/haible/include -Wall -fvisibility=hidden -g -O2 -MT jitterc/bin_jitter-jitterc-generate.o -MD -MP -MF jitterc/.deps/bin_jitter-jitterc-generate.Tpo -c -o jitterc/bin_jitter-jitterc-generate.o `test -f 'jitterc/jitterc-generate.c' || echo '../../jitter/'`jitterc/jitterc-generate.c
In file included from ../../jitter/jitterc/jitterc-utility.h:27,
                 from ../../jitter/jitterc/jitterc-generate.c:39:
./config-private/config.h:2114:1: error: macro "__has_attribute" requires an identifier
 2114 | #if _GL_HAS_ATTRIBUTE (returns_nonnull)
      | ^~~~~~~~~~~~~~~~~~~
./config-private/config.h:2117: warning: "_GL_ATTRIBUTE_RETURNS_NONNULL" redefined
 2117 | # define _GL_ATTRIBUTE_RETURNS_NONNULL
      | 
In file included from ../../jitter/jitterc/jitterc-vm.h:26,
                 from ../../jitter/jitterc/jitterc-generate.h:33,
                 from ../../jitter/jitterc/jitterc-generate.c:38:
./config-private/config.h:2115: note: this is the location of the previous definition
 2115 | # define _GL_ATTRIBUTE_RETURNS_NONNULL __attribute__ ((__returns_nonnull__))
      | 
In file included from ../../jitter/jitterc/jitterc-rewrite.h:26,
                 from ../../jitter/jitterc/jitterc-generate.c:41:
./config-private/config.h:2114:1: error: macro "__has_attribute" requires an identifier
 2114 | #if _GL_HAS_ATTRIBUTE (returns_nonnull)
      | ^~~~~~~~~~~~~~~~~~~
make[3]: *** [Makefile:10494: jitterc/bin_jitter-jitterc-generate.o] Error 1
make[3]: Leaving directory '/home/haible/poke-2.90.1/build/jitter'
make[2]: *** [Makefile:18650: check-recursive] Error 1

What happens, is that this compilation unit #includes <config.h> five times,
and somewhere between the first and the fifth inclusion, poke's code does
  #define returns_nonnull

This is their particular way of not emitting this particular GCC/clang
attribute, and is OK since 'returns_nonnull' is not a keyword and not a
token in the reserved namespace.

Including <config.h> several times is OK to do, according to our documentation
in https://www.gnu.org/software/gnulib/manual/html_node/Source-changes.html :

  - We say "to all your source files and likely also to all your tests source
    files you need to add an ‘#include <config.h>’ at the top."
    The GNU jitter authors took that literally and added '#include <config.h>'
    not only at the top of the .c files, but also at the top of the .h files.

  - We don't say that #include <config.h> should not be placed at the top of
    .h files. Doing so is an optimization, and we don't force (nor even hint)
    our users at this optimization.

Hence this sequence

  #include <config.h>
  #define returns_nonnull
  #include <config.h>

is supposed to work fine. But it produces an error

  error: macro "__has_attribute" requires an identifier


This patch fixes it, by adding double-inclusion guards to all code that uses
__has_attribute. Note the we had already done so for _GL_ATTRIBUTE_DEALLOC_FREE.


2023-01-25  Bruno Haible  <bruno@clisp.org>

	Keep config.h idempotent.
	* m4/gnulib-common.m4 (gl_COMMON_BODY): Add double-inclusion guards to
	the definitions of the macros _GL_ATTRIBUTE_ALLOC_SIZE,
	_GL_ATTRIBUTE_ALWAYS_INLINE, _GL_ATTRIBUTE_ARTIFICIAL,
	_GL_ATTRIBUTE_COLD, _GL_ATTRIBUTE_CONST, _GL_ATTRIBUTE_DEALLOC,
	_GL_ATTRIBUTE_DEPRECATED, _GL_ATTRIBUTE_ERROR, _GL_ATTRIBUTE_WARNING,
	_GL_ATTRIBUTE_EXTERNALLY_VISIBLE, _GL_ATTRIBUTE_FALLTHROUGH,
	_GL_ATTRIBUTE_FORMAT, _GL_ATTRIBUTE_LEAF, _GL_ATTRIBUTE_MALLOC,
	_GL_ATTRIBUTE_MAY_ALIAS, _GL_ATTRIBUTE_MAYBE_UNUSED,
	_GL_ATTRIBUTE_NODISCARD, _GL_ATTRIBUTE_NOINLINE, _GL_ATTRIBUTE_NONNULL,
	_GL_ATTRIBUTE_NONSTRING, _GL_ATTRIBUTE_NOTHROW, _GL_ATTRIBUTE_PACKED,
	_GL_ATTRIBUTE_PURE, _GL_ATTRIBUTE_RETURNS_NONNULL,
	_GL_ATTRIBUTE_SENTINEL, _GL_ATTRIBUTE_UNUSED, _GL_UNUSED_LABEL.

diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
index d5b2f7c4e5..0f1a6ac1ea 100644
--- a/m4/gnulib-common.m4
+++ b/m4/gnulib-common.m4
@@ -1,4 +1,4 @@
-# gnulib-common.m4 serial 77
+# gnulib-common.m4 serial 78
 dnl Copyright (C) 2007-2023 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -124,29 +124,35 @@ AC_DEFUN([gl_COMMON_BODY], [
    by the Nth argument of the function is the size of the returned memory block.
  */
 /* Applies to: function, pointer to function, function types.  */
-#if _GL_HAS_ATTRIBUTE (alloc_size)
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
-#else
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args)
+#ifndef _GL_ATTRIBUTE_ALLOC_SIZE
+# if _GL_HAS_ATTRIBUTE (alloc_size)
+#  define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
+# else
+#  define _GL_ATTRIBUTE_ALLOC_SIZE(args)
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_ALWAYS_INLINE tells that the compiler should always inline the
    function and report an error if it cannot do so.  */
 /* Applies to: function.  */
-#if _GL_HAS_ATTRIBUTE (always_inline)
-# define _GL_ATTRIBUTE_ALWAYS_INLINE __attribute__ ((__always_inline__))
-#else
-# define _GL_ATTRIBUTE_ALWAYS_INLINE
+#ifndef _GL_ATTRIBUTE_ALWAYS_INLINE
+# if _GL_HAS_ATTRIBUTE (always_inline)
+#  define _GL_ATTRIBUTE_ALWAYS_INLINE __attribute__ ((__always_inline__))
+# else
+#  define _GL_ATTRIBUTE_ALWAYS_INLINE
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_ARTIFICIAL declares that the function is not important to show
     in stack traces when debugging.  The compiler should omit the function from
     stack traces.  */
 /* Applies to: function.  */
-#if _GL_HAS_ATTRIBUTE (artificial)
-# define _GL_ATTRIBUTE_ARTIFICIAL __attribute__ ((__artificial__))
-#else
-# define _GL_ATTRIBUTE_ARTIFICIAL
+#ifndef _GL_ATTRIBUTE_ARTIFICIAL
+# if _GL_HAS_ATTRIBUTE (artificial)
+#  define _GL_ATTRIBUTE_ARTIFICIAL __attribute__ ((__artificial__))
+# else
+#  define _GL_ATTRIBUTE_ARTIFICIAL
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_COLD declares that the function is rarely executed.  */
@@ -154,14 +160,16 @@ AC_DEFUN([gl_COMMON_BODY], [
 /* Avoid __attribute__ ((cold)) on MinGW; see thread starting at
    <https://lists.gnu.org/r/emacs-devel/2019-04/msg01152.html>.
    Also, Oracle Studio 12.6 requires 'cold' not '__cold__'.  */
-#if _GL_HAS_ATTRIBUTE (cold) && !defined __MINGW32__
-# ifndef __SUNPRO_C
-#  define _GL_ATTRIBUTE_COLD __attribute__ ((__cold__))
+#ifndef _GL_ATTRIBUTE_COLD
+# if _GL_HAS_ATTRIBUTE (cold) && !defined __MINGW32__
+#  ifndef __SUNPRO_C
+#   define _GL_ATTRIBUTE_COLD __attribute__ ((__cold__))
+#  else
+#   define _GL_ATTRIBUTE_COLD __attribute__ ((cold))
+#  endif
 # else
-#  define _GL_ATTRIBUTE_COLD __attribute__ ((cold))
+#  define _GL_ATTRIBUTE_COLD
 # endif
-#else
-# define _GL_ATTRIBUTE_COLD
 #endif
 
 /* _GL_ATTRIBUTE_CONST declares that it is OK for a compiler to omit duplicate
@@ -171,10 +179,12 @@ AC_DEFUN([gl_COMMON_BODY], [
    forever, and does not call longjmp.
    (This attribute is stricter than _GL_ATTRIBUTE_PURE.)  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (const)
-# define _GL_ATTRIBUTE_CONST __attribute__ ((__const__))
-#else
-# define _GL_ATTRIBUTE_CONST
+#ifndef _GL_ATTRIBUTE_CONST
+# if _GL_HAS_ATTRIBUTE (const)
+#  define _GL_ATTRIBUTE_CONST __attribute__ ((__const__))
+# else
+#  define _GL_ATTRIBUTE_CONST
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_DEALLOC (F, I) declares that the function returns pointers
@@ -183,10 +193,12 @@ AC_DEFUN([gl_COMMON_BODY], [
    _GL_ATTRIBUTE_DEALLOC_FREE declares that the function returns pointers that
    can be freed via 'free'; it can be used only after declaring 'free'.  */
 /* Applies to: functions.  Cannot be used on inline functions.  */
-#if _GL_GNUC_PREREQ (11, 0)
-# define _GL_ATTRIBUTE_DEALLOC(f, i) __attribute__ ((__malloc__ (f, i)))
-#else
-# define _GL_ATTRIBUTE_DEALLOC(f, i)
+#ifndef _GL_ATTRIBUTE_DEALLOC
+# if _GL_GNUC_PREREQ (11, 0)
+#  define _GL_ATTRIBUTE_DEALLOC(f, i) __attribute__ ((__malloc__ (f, i)))
+# else
+#  define _GL_ATTRIBUTE_DEALLOC(f, i)
+# endif
 #endif
 /* If gnulib's <string.h> or <wchar.h> has already defined this macro, continue
    to use this earlier definition, since <stdlib.h> may not have been included
@@ -210,16 +222,18 @@ AC_DEFUN([gl_COMMON_BODY], [
      - enumeration, enumeration item,
      - typedef,
    in C++ also: namespace, class, template specialization.  */
-#ifdef __has_c_attribute
-# if __has_c_attribute (__deprecated__)
-#  define _GL_ATTRIBUTE_DEPRECATED [[__deprecated__]]
-# endif
-#endif
-#if !defined _GL_ATTRIBUTE_DEPRECATED && _GL_HAS_ATTRIBUTE (deprecated)
-# define _GL_ATTRIBUTE_DEPRECATED __attribute__ ((__deprecated__))
-#endif
 #ifndef _GL_ATTRIBUTE_DEPRECATED
-# define _GL_ATTRIBUTE_DEPRECATED
+# ifdef __has_c_attribute
+#  if __has_c_attribute (__deprecated__)
+#   define _GL_ATTRIBUTE_DEPRECATED [[__deprecated__]]
+#  endif
+# endif
+# if !defined _GL_ATTRIBUTE_DEPRECATED && _GL_HAS_ATTRIBUTE (deprecated)
+#  define _GL_ATTRIBUTE_DEPRECATED __attribute__ ((__deprecated__))
+# endif
+# ifndef _GL_ATTRIBUTE_DEPRECATED
+#  define _GL_ATTRIBUTE_DEPRECATED
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_ERROR(msg) requests an error if a function is called and
@@ -227,24 +241,28 @@ AC_DEFUN([gl_COMMON_BODY], [
    _GL_ATTRIBUTE_WARNING(msg) requests a warning if a function is called and
    the function call is not optimized away.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (error)
-# define _GL_ATTRIBUTE_ERROR(msg) __attribute__ ((__error__ (msg)))
-# define _GL_ATTRIBUTE_WARNING(msg) __attribute__ ((__warning__ (msg)))
-#elif _GL_HAS_ATTRIBUTE (diagnose_if)
-# define _GL_ATTRIBUTE_ERROR(msg) __attribute__ ((__diagnose_if__ (1, msg, "error")))
-# define _GL_ATTRIBUTE_WARNING(msg) __attribute__ ((__diagnose_if__ (1, msg, "warning")))
-#else
-# define _GL_ATTRIBUTE_ERROR(msg)
-# define _GL_ATTRIBUTE_WARNING(msg)
+#if !(defined _GL_ATTRIBUTE_ERROR && defined _GL_ATTRIBUTE_WARNING)
+# if _GL_HAS_ATTRIBUTE (error)
+#  define _GL_ATTRIBUTE_ERROR(msg) __attribute__ ((__error__ (msg)))
+#  define _GL_ATTRIBUTE_WARNING(msg) __attribute__ ((__warning__ (msg)))
+# elif _GL_HAS_ATTRIBUTE (diagnose_if)
+#  define _GL_ATTRIBUTE_ERROR(msg) __attribute__ ((__diagnose_if__ (1, msg, "error")))
+#  define _GL_ATTRIBUTE_WARNING(msg) __attribute__ ((__diagnose_if__ (1, msg, "warning")))
+# else
+#  define _GL_ATTRIBUTE_ERROR(msg)
+#  define _GL_ATTRIBUTE_WARNING(msg)
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_EXTERNALLY_VISIBLE declares that the entity should remain
    visible to debuggers etc., even with '-fwhole-program'.  */
 /* Applies to: functions, variables.  */
-#if _GL_HAS_ATTRIBUTE (externally_visible)
-# define _GL_ATTRIBUTE_EXTERNALLY_VISIBLE __attribute__ ((externally_visible))
-#else
-# define _GL_ATTRIBUTE_EXTERNALLY_VISIBLE
+#ifndef _GL_ATTRIBUTE_EXTERNALLY_VISIBLE
+# if _GL_HAS_ATTRIBUTE (externally_visible)
+#  define _GL_ATTRIBUTE_EXTERNALLY_VISIBLE __attribute__ ((externally_visible))
+# else
+#  define _GL_ATTRIBUTE_EXTERNALLY_VISIBLE
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_FALLTHROUGH declares that it is not a programming mistake if
@@ -252,16 +270,18 @@ AC_DEFUN([gl_COMMON_BODY], [
    'default' label.  The compiler should not warn in this case.  */
 /* Applies to: Empty statement (;), inside a 'switch' statement.  */
 /* Always expands to something.  */
-#ifdef __has_c_attribute
-# if __has_c_attribute (__fallthrough__)
-#  define _GL_ATTRIBUTE_FALLTHROUGH [[__fallthrough__]]
-# endif
-#endif
-#if !defined _GL_ATTRIBUTE_FALLTHROUGH && _GL_HAS_ATTRIBUTE (fallthrough)
-# define _GL_ATTRIBUTE_FALLTHROUGH __attribute__ ((__fallthrough__))
-#endif
 #ifndef _GL_ATTRIBUTE_FALLTHROUGH
-# define _GL_ATTRIBUTE_FALLTHROUGH ((void) 0)
+# ifdef __has_c_attribute
+#  if __has_c_attribute (__fallthrough__)
+#   define _GL_ATTRIBUTE_FALLTHROUGH [[__fallthrough__]]
+#  endif
+# endif
+# if !defined _GL_ATTRIBUTE_FALLTHROUGH && _GL_HAS_ATTRIBUTE (fallthrough)
+#  define _GL_ATTRIBUTE_FALLTHROUGH __attribute__ ((__fallthrough__))
+# endif
+# ifndef _GL_ATTRIBUTE_FALLTHROUGH
+#  define _GL_ATTRIBUTE_FALLTHROUGH ((void) 0)
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_FORMAT ((ARCHETYPE, STRING-INDEX, FIRST-TO-CHECK))
@@ -275,10 +295,12 @@ AC_DEFUN([gl_COMMON_BODY], [
    If FIRST-TO-CHECK is not 0, arguments starting at FIRST-TO_CHECK
    are suitable for the format string.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (format)
-# define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec))
-#else
-# define _GL_ATTRIBUTE_FORMAT(spec)
+#ifndef _GL_ATTRIBUTE_FORMAT
+# if _GL_HAS_ATTRIBUTE (format)
+#  define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec))
+# else
+#  define _GL_ATTRIBUTE_FORMAT(spec)
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_LEAF declares that if the function is called from some other
@@ -286,19 +308,23 @@ AC_DEFUN([gl_COMMON_BODY], [
    exception handling.  This declaration lets the compiler optimize that unit
    more aggressively.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (leaf)
-# define _GL_ATTRIBUTE_LEAF __attribute__ ((__leaf__))
-#else
-# define _GL_ATTRIBUTE_LEAF
+#ifndef _GL_ATTRIBUTE_LEAF
+# if _GL_HAS_ATTRIBUTE (leaf)
+#  define _GL_ATTRIBUTE_LEAF __attribute__ ((__leaf__))
+# else
+#  define _GL_ATTRIBUTE_LEAF
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_MALLOC declares that the function returns a pointer to freshly
    allocated memory.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (malloc)
-# define _GL_ATTRIBUTE_MALLOC __attribute__ ((__malloc__))
-#else
-# define _GL_ATTRIBUTE_MALLOC
+#ifndef _GL_ATTRIBUTE_MALLOC
+# if _GL_HAS_ATTRIBUTE (malloc)
+#  define _GL_ATTRIBUTE_MALLOC __attribute__ ((__malloc__))
+# else
+#  define _GL_ATTRIBUTE_MALLOC
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_MAY_ALIAS declares that pointers to the type may point to the
@@ -306,10 +332,12 @@ AC_DEFUN([gl_COMMON_BODY], [
    strict aliasing optimization.  */
 /* Applies to: types.  */
 /* Oracle Studio 12.6 mishandles may_alias despite __has_attribute OK.  */
-#if _GL_HAS_ATTRIBUTE (may_alias) && !defined __SUNPRO_C
-# define _GL_ATTRIBUTE_MAY_ALIAS __attribute__ ((__may_alias__))
-#else
-# define _GL_ATTRIBUTE_MAY_ALIAS
+#ifndef _GL_ATTRIBUTE_MAY_ALIAS
+# if _GL_HAS_ATTRIBUTE (may_alias) && !defined __SUNPRO_C
+#  define _GL_ATTRIBUTE_MAY_ALIAS __attribute__ ((__may_alias__))
+# else
+#  define _GL_ATTRIBUTE_MAY_ALIAS
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_MAYBE_UNUSED declares that it is not a programming mistake if
@@ -324,13 +352,15 @@ AC_DEFUN([gl_COMMON_BODY], [
 /* In C++ and C23, this is spelled [[__maybe_unused__]].
    GCC's syntax is __attribute__ ((__unused__)).
    clang supports both syntaxes.  */
-#ifdef __has_c_attribute
-# if __has_c_attribute (__maybe_unused__)
-#  define _GL_ATTRIBUTE_MAYBE_UNUSED [[__maybe_unused__]]
-# endif
-#endif
 #ifndef _GL_ATTRIBUTE_MAYBE_UNUSED
-# define _GL_ATTRIBUTE_MAYBE_UNUSED _GL_ATTRIBUTE_UNUSED
+# ifdef __has_c_attribute
+#  if __has_c_attribute (__maybe_unused__)
+#   define _GL_ATTRIBUTE_MAYBE_UNUSED [[__maybe_unused__]]
+#  endif
+# endif
+# ifndef _GL_ATTRIBUTE_MAYBE_UNUSED
+#  define _GL_ATTRIBUTE_MAYBE_UNUSED _GL_ATTRIBUTE_UNUSED
+# endif
 #endif
 /* Alternative spelling of this macro, for convenience and for
    compatibility with glibc/include/libc-symbols.h.  */
@@ -342,25 +372,29 @@ AC_DEFUN([gl_COMMON_BODY], [
    discard the return value.  The compiler may warn if the caller does not use
    the return value, unless the caller uses something like ignore_value.  */
 /* Applies to: function, enumeration, class.  */
-#ifdef __has_c_attribute
-# if __has_c_attribute (__nodiscard__)
-#  define _GL_ATTRIBUTE_NODISCARD [[__nodiscard__]]
-# endif
-#endif
-#if !defined _GL_ATTRIBUTE_NODISCARD && _GL_HAS_ATTRIBUTE (warn_unused_result)
-# define _GL_ATTRIBUTE_NODISCARD __attribute__ ((__warn_unused_result__))
-#endif
 #ifndef _GL_ATTRIBUTE_NODISCARD
-# define _GL_ATTRIBUTE_NODISCARD
+# ifdef __has_c_attribute
+#  if __has_c_attribute (__nodiscard__)
+#   define _GL_ATTRIBUTE_NODISCARD [[__nodiscard__]]
+#  endif
+# endif
+# if !defined _GL_ATTRIBUTE_NODISCARD && _GL_HAS_ATTRIBUTE (warn_unused_result)
+#  define _GL_ATTRIBUTE_NODISCARD __attribute__ ((__warn_unused_result__))
+# endif
+# ifndef _GL_ATTRIBUTE_NODISCARD
+#  define _GL_ATTRIBUTE_NODISCARD
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_NOINLINE tells that the compiler should not inline the
    function.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (noinline)
-# define _GL_ATTRIBUTE_NOINLINE __attribute__ ((__noinline__))
-#else
-# define _GL_ATTRIBUTE_NOINLINE
+#ifndef _GL_ATTRIBUTE_NOINLINE
+# if _GL_HAS_ATTRIBUTE (noinline)
+#  define _GL_ATTRIBUTE_NOINLINE __attribute__ ((__noinline__))
+# else
+#  define _GL_ATTRIBUTE_NOINLINE
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_NONNULL ((N1, N2,...)) declares that the arguments N1, N2,...
@@ -368,20 +402,24 @@ AC_DEFUN([gl_COMMON_BODY], [
    _GL_ATTRIBUTE_NONNULL () declares that all pointer arguments must not be
    null.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (nonnull)
-# define _GL_ATTRIBUTE_NONNULL(args) __attribute__ ((__nonnull__ args))
-#else
-# define _GL_ATTRIBUTE_NONNULL(args)
+#ifndef _GL_ATTRIBUTE_NONNULL
+# if _GL_HAS_ATTRIBUTE (nonnull)
+#  define _GL_ATTRIBUTE_NONNULL(args) __attribute__ ((__nonnull__ args))
+# else
+#  define _GL_ATTRIBUTE_NONNULL(args)
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_NONSTRING declares that the contents of a character array is
    not meant to be NUL-terminated.  */
 /* Applies to: struct/union members and variables that are arrays of element
    type '[[un]signed] char'.  */
-#if _GL_HAS_ATTRIBUTE (nonstring)
-# define _GL_ATTRIBUTE_NONSTRING __attribute__ ((__nonstring__))
-#else
-# define _GL_ATTRIBUTE_NONSTRING
+#ifndef _GL_ATTRIBUTE_NONSTRING
+# if _GL_HAS_ATTRIBUTE (nonstring)
+#  define _GL_ATTRIBUTE_NONSTRING __attribute__ ((__nonstring__))
+# else
+#  define _GL_ATTRIBUTE_NONSTRING
+# endif
 #endif
 
 /* There is no _GL_ATTRIBUTE_NORETURN; use _Noreturn instead.  */
@@ -389,10 +427,12 @@ AC_DEFUN([gl_COMMON_BODY], [
 /* _GL_ATTRIBUTE_NOTHROW declares that the function does not throw exceptions.
  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (nothrow) && !defined __cplusplus
-# define _GL_ATTRIBUTE_NOTHROW __attribute__ ((__nothrow__))
-#else
-# define _GL_ATTRIBUTE_NOTHROW
+#ifndef _GL_ATTRIBUTE_NOTHROW
+# if _GL_HAS_ATTRIBUTE (nothrow) && !defined __cplusplus
+#  define _GL_ATTRIBUTE_NOTHROW __attribute__ ((__nothrow__))
+# else
+#  define _GL_ATTRIBUTE_NOTHROW
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_PACKED declares:
@@ -401,10 +441,12 @@ AC_DEFUN([gl_COMMON_BODY], [
    minimizing the memory required.  */
 /* Applies to: struct members, struct, union,
    in C++ also: class.  */
-#if _GL_HAS_ATTRIBUTE (packed)
-# define _GL_ATTRIBUTE_PACKED __attribute__ ((__packed__))
-#else
-# define _GL_ATTRIBUTE_PACKED
+#ifndef _GL_ATTRIBUTE_PACKED
+# if _GL_HAS_ATTRIBUTE (packed)
+#  define _GL_ATTRIBUTE_PACKED __attribute__ ((__packed__))
+# else
+#  define _GL_ATTRIBUTE_PACKED
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_PURE declares that It is OK for a compiler to omit duplicate
@@ -414,19 +456,23 @@ AC_DEFUN([gl_COMMON_BODY], [
    observable state, and always returns exactly once.
    (This attribute is looser than _GL_ATTRIBUTE_CONST.)  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (pure)
-# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
-#else
-# define _GL_ATTRIBUTE_PURE
+#ifndef _GL_ATTRIBUTE_PURE
+# if _GL_HAS_ATTRIBUTE (pure)
+#  define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
+# else
+#  define _GL_ATTRIBUTE_PURE
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_RETURNS_NONNULL declares that the function's return value is
    a non-NULL pointer.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (returns_nonnull)
-# define _GL_ATTRIBUTE_RETURNS_NONNULL __attribute__ ((__returns_nonnull__))
-#else
-# define _GL_ATTRIBUTE_RETURNS_NONNULL
+#ifndef _GL_ATTRIBUTE_RETURNS_NONNULL
+# if _GL_HAS_ATTRIBUTE (returns_nonnull)
+#  define _GL_ATTRIBUTE_RETURNS_NONNULL __attribute__ ((__returns_nonnull__))
+# else
+#  define _GL_ATTRIBUTE_RETURNS_NONNULL
+# endif
 #endif
 
 /* _GL_ATTRIBUTE_SENTINEL(pos) declares that the variadic function expects a
@@ -434,17 +480,21 @@ AC_DEFUN([gl_COMMON_BODY], [
    _GL_ATTRIBUTE_SENTINEL () - The last argument is NULL (requires C99).
    _GL_ATTRIBUTE_SENTINEL ((N)) - The (N+1)st argument from the end is NULL.  */
 /* Applies to: functions.  */
-#if _GL_HAS_ATTRIBUTE (sentinel)
-# define _GL_ATTRIBUTE_SENTINEL(pos) __attribute__ ((__sentinel__ pos))
-#else
-# define _GL_ATTRIBUTE_SENTINEL(pos)
+#ifndef _GL_ATTRIBUTE_SENTINEL
+# if _GL_HAS_ATTRIBUTE (sentinel)
+#  define _GL_ATTRIBUTE_SENTINEL(pos) __attribute__ ((__sentinel__ pos))
+# else
+#  define _GL_ATTRIBUTE_SENTINEL(pos)
+# endif
 #endif
 
 /* A helper macro.  Don't use it directly.  */
-#if _GL_HAS_ATTRIBUTE (unused)
-# define _GL_ATTRIBUTE_UNUSED __attribute__ ((__unused__))
-#else
-# define _GL_ATTRIBUTE_UNUSED
+#ifndef _GL_ATTRIBUTE_UNUSED
+# if _GL_HAS_ATTRIBUTE (unused)
+#  define _GL_ATTRIBUTE_UNUSED __attribute__ ((__unused__))
+# else
+#  define _GL_ATTRIBUTE_UNUSED
+# endif
 #endif
 
 ]dnl There is no _GL_ATTRIBUTE_VISIBILITY; see m4/visibility.m4 instead.
@@ -455,10 +505,12 @@ AC_DEFUN([gl_COMMON_BODY], [
 /* Applies to: label (both in C and C++).  */
 /* Note that g++ < 4.5 does not support the '__attribute__ ((__unused__)) ;'
    syntax.  But clang does.  */
-#if !(defined __cplusplus && !_GL_GNUC_PREREQ (4, 5)) || defined __clang__
-# define _GL_UNUSED_LABEL _GL_ATTRIBUTE_UNUSED
-#else
-# define _GL_UNUSED_LABEL
+#ifndef _GL_UNUSED_LABEL
+# if !(defined __cplusplus && !_GL_GNUC_PREREQ (4, 5)) || defined __clang__
+#  define _GL_UNUSED_LABEL _GL_ATTRIBUTE_UNUSED
+# else
+#  define _GL_UNUSED_LABEL
+# endif
 #endif
 ])
   AH_VERBATIM([async_safe],





^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: Keep config.h idempotent
  2023-01-25 14:05 Keep config.h idempotent Bruno Haible
@ 2023-01-25 22:00 ` Paul Eggert
  2023-01-25 23:48   ` Bruno Haible
  0 siblings, 1 reply; 4+ messages in thread
From: Paul Eggert @ 2023-01-25 22:00 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib, epsilon-devel; +Cc: poke-devel

On 2023-01-25 06:05, Bruno Haible wrote:
> What happens, is that this compilation unit #includes <config.h> five times,
> and somewhere between the first and the fifth inclusion, poke's code does
>    #define returns_nonnull
> 
> This is their particular way of not emitting this particular GCC/clang
> attribute, and is OK since 'returns_nonnull' is not a keyword and not a
> token in the reserved namespace.

Is this diagnosis correct? It appears to me that poke 2.90.1's "#define 
returns_nonnull" is not a problem, because the Gnulib code never uses 
that identifier. Instead, the Gnulib code does this:

#if _GL_HAS_ATTRIBUTE (returns_nonnull)

which uses this macro:

# define _GL_HAS_ATTRIBUTE(attr) __has_attribute (__##attr##__)

and so has the same effect as this:

#if __has_attribute (__returns_nonnull__)

regardless of what returns_nonnull was defined to, because C doesn't 
macro-expand immediate operands of ##.

If I understand things correctly the problem with poke 2.90.1 wasn't its 
"#define returns_nonnull"; it was its "#define __returns_nonnull__", 
which indeed defines a token in the reserved namespace.

> Hence this sequence
> 
>   #include <config.h>
>   #define returns_nonnull
>   #include <config.h>
> 
> is supposed to work fine.

Isn't there still a problem with something like the following, even with 
the latest Gnulib?

    #define __returns_nonnull__
    #include <config.h>

That is, it seems to me that the real problem here is a collision with 
poke's "#define __returns_nonnull__" before <config.h>, not with 
including <config.h> twice.

Obviously we can't make Gnulib immune to arbitrary #defines of reserved 
names. This is not to say that we shouldn't accommodate GNU poke, just 
that it's not clear that the cure is right here, as a general rule.

More generally, I don't think Gnulib should imply to users that it's OK 
to include <config.h> twice. That's an uncommon practice and is a recipe 
for trouble, as too many programs do fun stuff in their configure.ac 
that Gnulib has no control over. Instead, we should clarify Gnulib's 
documentation to say that compilation units should include <config.h> 
first, before including anything else, and should not include <config.h> 
later.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Keep config.h idempotent
  2023-01-25 22:00 ` Paul Eggert
@ 2023-01-25 23:48   ` Bruno Haible
  2023-01-26  4:17     ` Paul Eggert
  0 siblings, 1 reply; 4+ messages in thread
From: Bruno Haible @ 2023-01-25 23:48 UTC (permalink / raw)
  To: bug-gnulib, epsilon-devel, Paul Eggert; +Cc: poke-devel

Hi Paul, Luca,

Paul Eggert wrote:
> It appears to me that poke 2.90.1's "#define 
> returns_nonnull" is not a problem, because the Gnulib code never uses 
> that identifier. Instead, the Gnulib code does this:
> 
> #if _GL_HAS_ATTRIBUTE (returns_nonnull)
> 
> which uses this macro:
> 
> # define _GL_HAS_ATTRIBUTE(attr) __has_attribute (__##attr##__)
> 
> and so has the same effect as this:
> 
> #if __has_attribute (__returns_nonnull__)
> 
> regardless of what returns_nonnull was defined to, because C doesn't 
> macro-expand immediate operands of ##.

Indeed, poke does

  # define returns_nonnull      /* Nothing. */
  # define __returns_nonnull__  /* Nothing. */

and, as you say, it's the latter that is troublesome.

Luca, while it would be good to remove this invalid
  # define __returns_nonnull__  /* Nothing. */
line in the long run, it's OK to leave it in for poke-3.0, since the Gnulib
patch that I committed, plus the gnulib submodule update that José committed,
will prevent the trouble.

> Isn't there still a problem with something like the following, even with 
> the latest Gnulib?
> 
>     #define __returns_nonnull__
>     #include <config.h>
> 
> That is, it seems to me that the real problem here is a collision with 
> poke's "#define __returns_nonnull__" before <config.h>, not with 
> including <config.h> twice.

True. Although it's rare that people will want to make such macro definitions
before '#include <config.h>'. It's more likely that packages do such macro
definitions deep in their own code, when it's seen after the first include of
<config.h>.

> I don't think Gnulib should imply to users that it's OK 
> to include <config.h> twice. That's an uncommon practice and is a recipe 
> for trouble, as too many programs do fun stuff in their configure.ac 
> that Gnulib has no control over. Instead, we should clarify Gnulib's 
> documentation to say that compilation units should include <config.h> 
> first, before including anything else, and should not include <config.h> 
> later.

I disagree for two reasons:

  * <config.h> is an invention of Autoconf, and the Autoconf documentation
    does not state that <config.h> should only be included once. It only
    states that it should be included before the first system header file,
    otherwise definitions such as '#define _GNU_SOURCE 1' are ineffective.
    Gnulib, as a user of Autoconf, should not cause additional constraints.
    In other words, <config.h> was meant to be idempotent from the beginning,
    and it's only a Gnulib glitch that caused it to be not idempotent in
    the face of some invalid "#define __returns_nonnull__".

  * Additional constraints for the user means more complicated maintenance.
    If developers have to distinguish .c files which are compiled as a
    separate compilation unit from .c files which are only included, their
    code becomes more fragile.
    And yes, some people do #include .c files, see e.g.
    gnulib/lib/strtoul.c or
    https://git.savannah.gnu.org/gitweb/?p=gettext.git;a=blob;f=gettext-runtime/libasprintf/lib-asprintf.c;h=c2fe4ed161f2716df3d5a96890154aa55c0fc656;hb=eb83efc3b58d690d444da0b82c3ecf36a54f28ac
    It's better to continue to give them the freedom of adding
    '#include <config.h>' where they feel a need for.
    The simpler the documentation is on this topic, the easier Gnulib can
    be used, and the fewer people curse Gnulib and Autoconf.

    In the case that you mentioned, that the developer has added AH_VERBATIM
    invocations in their configure.ac and that it destroys the idempotency
    of <config.h>, that's entirely their problem. The Gnulib or Autoconf
    documentation should not force practices on the user that are entirely
    in the domain / responsibility of the user.

> Obviously we can't make Gnulib immune to arbitrary #defines of reserved 
> names. This is not to say that we shouldn't accommodate GNU poke, just 
> that it's not clear that the cure is right here, as a general rule.

The change that I committed had two effects:
  (1) Work around that arbitrary #define.
  (2) Allow to override these _GL_ATTRIBUTE_* macros for the scope of a
      compilation unit. We already have seen the need to override
      _GL_ARG_NONNULL at compilation-unit scope. Similar needs may arise
      for any _GL_ATTRIBUTE_* macro.

So, I'd like to keep this commit, even if the rationale (1) is debatable.

Bruno





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Keep config.h idempotent
  2023-01-25 23:48   ` Bruno Haible
@ 2023-01-26  4:17     ` Paul Eggert
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Eggert @ 2023-01-26  4:17 UTC (permalink / raw)
  To: Bruno Haible, bug-gnulib, epsilon-devel; +Cc: poke-devel

On 1/25/23 15:48, Bruno Haible wrote:
>    (2) Allow to override these_GL_ATTRIBUTE_* macros for the scope of a
>        compilation unit. We already have seen the need to override
>        _GL_ARG_NONNULL at compilation-unit scope. Similar needs may arise
>        for any_GL_ATTRIBUTE_* macro.

Yes, good point, and thanks for looking into all this.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-01-26  4:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 14:05 Keep config.h idempotent Bruno Haible
2023-01-25 22:00 ` Paul Eggert
2023-01-25 23:48   ` Bruno Haible
2023-01-26  4:17     ` Paul Eggert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).