Re: gcc -fanalyze

[Paul Eggert]

The first bug in that output:

cc1: warning: function may return address of local variable 
[-Wreturn-local-addr]
lib/careadlinkat.c:73:8: note: declared here
   73 |   char stack_buf[1024];
  |^

This is a false alarm. I installed the attached patch to pacify GCC (if you
build with GCC_LINT).

But perhaps I jumped the gun here - the workaround is reasonably awful, so
perhaps it'd be better to disable -Wreturn-local-addr instead, at least for this
compilation unit.

lib/diffseq.h:432:36: warning: 'bxbest' may be used uninitialized in this
function [-Wmaybe-uninitialized]
  432 |   part->ymid = bxybest - bxbest;
  |^~~~

This false alarm is because the program was built without GCC_LINT being
defined. If you build with -DGCC_LINT the false alarm should go away. (Maybe
some others will go away too, at least careadlinkat is like this now) This
is what Emacs etc. do. Perhaps this should be moved to the manywarnings module?

Speaking of which, manywarnings.m4 should be updated for GCC 10, mostly for the
-fanalyze warnings. I installed the second attached patch to do that.
>From 26f82c14e89f71ab1c2e6a811981ecc742b044e5 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 9 May 2020 18:01:59 -0700
Subject: [PATCH 1/2] careadlinkat: pacify -Wreturn-local-addr
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/careadlinkat.c (careadlinkat) [GCC_LINT]:
Pacify gcc 10’s -Wreturn-local-addr option.
Simplify some of the later code.
---
 ChangeLog  |  5 +
 lib/careadlinkat.c | 29 +++--
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c35cfee97..b6d070599 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2020-05-09  Paul Eggert  
 
+	careadlinkat: pacify -Wreturn-local-addr
+	* lib/careadlinkat.c (careadlinkat) [GCC_LINT]:
+	Pacify gcc 10’s -Wreturn-local-addr option.
+	Simplify some of the later code.
+
 	attribute: remove ATTRIBUTE_DEPRECATED
 	* lib/attribute.h: Improve recently-added comments, mostly
 	by shortening them (use active voice, etc.).
diff --git a/lib/careadlinkat.c b/lib/careadlinkat.c
index 1effdb784..e1f8305e9 100644
--- a/lib/careadlinkat.c
+++ b/lib/careadlinkat.c
@@ -70,19 +70,28 @@ careadlinkat (int fd, char const *filename,
   size_t buf_size;
   size_t buf_size_max =
 SSIZE_MAX < SIZE_MAX ? (size_t) SSIZE_MAX + 1 : SIZE_MAX;
-  char stack_buf[1024];
+
+#if defined GCC_LINT || defined lint
+  /* Pacify preadlinkat without creating a pointer to the stack
+ that gcc -Wreturn-local-addr would cry wolf about.  */
+  static char initial_buf[1];
+  enum { initial_buf_size = 0 }; /* 0 so that initial_buf never changes.  */
+#else
+  char initial_buf[1024];
+  enum { initial_buf_size = sizeof initial_buf };
+#endif
 
   if (! alloc)
 alloc = _allocator;
 
   if (! buffer_size)
 {
-  /* Allocate the initial buffer on the stack.  This way, in the
- common case of a symlink of small size, we get away with a
+  /* Allocate the initial buffer.  This way, in the common case of
+ a symlink of small size without GCC_LINT, we get away with a
  single small malloc() instead of a big malloc() followed by a
  shrinking realloc().  */
-  buffer = stack_buf;
-  buffer_size = sizeof stack_buf;
+  buffer = initial_buf;
+  buffer_size = initial_buf_size;
 }
 
   buf = buffer;
@@ -115,21 +124,21 @@ careadlinkat (int fd, char const *filename,
 {
   buf[link_size++] = '\0';
 
-  if (buf == stack_buf)
+  if (buf == initial_buf)
 {
   char *b = (char *) alloc->allocate (link_size);
   buf_size = link_size;
   if (! b)
 break;
-  memcpy (b, buf, link_size);
-  buf = b;
+  return memcpy (b, buf, link_size);
 }
-  else if (link_size < buf_size && buf != buffer && alloc->reallocate)
+
+  if (link_size < buf_size && buf != buffer && alloc->reallocate)
 {
   /* Shrink BUF before returning it.  */
   char *b = (char *) alloc->reallocate (buf, link_size);
   if (b)
-buf = b;
+return b;
 }
 
   return buf;
-- 
2.17.1

>From da2a958f184bfeb1d3491b53eed6473241d9268c Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 9 May 2020 18:07:38 -0700
Subject: [PATCH 2/2] manywarnings: port to GCC 10.1

* build-aux/gcc-warning.spec:
* m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC(C)):
Add GCC 10.1.0 warnings.
---
 ChangeLog  |  5 +
 build-aux/gcc-warning.spec | 14 --
 m4/manywarnings.m4 | 24 +++-
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b6d070599..b31bbb185 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ 

gcc -fanalyze

[Bruno Haible]

Hi all, volunteers,

GCC 10 has a new static analysis pass [1][2]. I compiled a gnulib testdir with
CC="gcc -fanalyze". Find attached the part of the log that compiles the gllib/
directory. (The log of the gltests/ directory is uninteresting: here gcc reports
mostly cases where memory was allocated with malloc(), not xmalloc().)

If you can help distinguish valid findings from false reports, please report
your evaluations.

When you report a valid finding, you don't need to provide a fix; we can do
that.

Bruno

[1] https://developers.redhat.com/blog/2020/03/26/static-analysis-in-gcc-10/
[2] https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Static-Analyzer-Options.html


fanalyze-log.xz
Description: application/xz

Re: m4: use AC_SUBST with two arguments when applicable

[Bruno Haible]

Hi Akim,

(A) > I'm looking for means to avoid repetitions, and boilerplate.

It needs to be balanced against the other characteristics of maintainability.
Two important aspects are:
  (B) Making the code easy to understand.
  (C) Making it easy to find all definitions and all references of an 
identifier.

One of the main tricks to avoid boilerplate is to create identifiers by
concatenation. For example, the variable GNULIB_ISWBLANK gets assigned through
gl_WCTYPE_MODULE_INDICATOR([iswblank]), which is not obvious. And in
gnulib-common.m4, it looks like _GL_ATTR_deprecated is never used. But in
fact, it is - though _GL_HAS_ATTRIBUTE. Such constructs avoid boilerplate,
but they make it hard to find all references of the identifiers.

On the other hand, what are the drawbacks of repetitive code?
  - Typos? Hardly. The maintainer should not type every identifier, but instead
use copy or use query on a template.
  - Difficult to grasp? Only if the indentation and comments don't help.
  - Too long to read? People now use editor windows with more than 50 rows.

In Lisp, the means of choice to avoid repetitions and boilerplate are the
macros. When I was at university, I loved macros (like DEFSTRUCT and DEFCLASS
in Lisp) that expand to interesting code. Until I realized that this works only
because
  - DEFSTRUCT and DEFCLASS have a detailed specification regarding the input
syntax and their effects,
  - most Lispers already know DEFSTRUCT and DEFCLASS, thus it does normally
not present much of a learning curve.
Until the day I had to debug a complex invocation of a complex, hand-crafted,
scarcely documented macro. I even had an IDE that showed me the result of the
macro-expansion when I modified the input; nevertheless, since then, I tend
to value (B) and (C) as more important than (A).

Bruno

Re: attribute: add comments

[Paul Eggert]

On 5/9/20 8:46 AM, Bruno Haible wrote:

> The text I wrote is meant be easier to understand upon first use, but
> still as correct (or nearly as correct) as the corresponding GCC 
> documentation.

Thanks, that's a good idea and makes attribute.h easier to use. I went through
the comments and tweaked them in a way that I hope makes them even clearer.

> Btw, why is there 'ATTRIBUTE_DEPRECATED', when there is also 'DEPRECATED',
> defined in the same file and with the same definition?

That's an oversight; I didn't notice the duplication. Thanks for mentioning it.

I installed the attached patch to implement the above.
>From 1e7b424f62d431fffc2ad1c7d84effc3314860c2 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 9 May 2020 12:00:08 -0700
Subject: [PATCH] attribute: remove ATTRIBUTE_DEPRECATED

* lib/attribute.h: Improve recently-added comments, mostly
by shortening them (use active voice, etc.).
(ATTRIBUTE_DEPRECATED): Remove, as it duplicates DEPRECATED.
Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-05/msg00089.html
---
 ChangeLog   |   9 +++
 lib/attribute.h | 161 +++-
 2 files changed, 74 insertions(+), 96 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e259be6e4..c35cfee97 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2020-05-09  Paul Eggert  
+
+	attribute: remove ATTRIBUTE_DEPRECATED
+	* lib/attribute.h: Improve recently-added comments, mostly
+	by shortening them (use active voice, etc.).
+	(ATTRIBUTE_DEPRECATED): Remove, as it duplicates DEPRECATED.
+	Problem reported by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2020-05/msg00089.html
+
 2020-05-09  Bruno Haible  
 
 	attribute: Add comments.
diff --git a/lib/attribute.h b/lib/attribute.h
index 22416a82f..3bd23faae 100644
--- a/lib/attribute.h
+++ b/lib/attribute.h
@@ -34,8 +34,7 @@
 /* C2X standard attributes have macro names that do not begin with
'ATTRIBUTE_'.  */
 
-/* Tells that the entity is deprecated.  A use of the entity produces a
-   warning.  */
+/* Warn if the entity is used.  */
 /* Applies to:
  - function, variable,
  - struct, union, struct/union member,
@@ -44,13 +43,12 @@
in C++ also: namespace, class, template specialization.  */
 #define DEPRECATED _GL_ATTRIBUTE_DEPRECATED
 
-/* Tells that it's intentional that the control flow can go on to the
-   immediately following 'case' or 'default' label.  */
+/* Do not warn if control flow falls through to the immediately
+   following 'case' or 'default' label.  */
 /* Applies to: Empty statement (;), inside a 'switch' statement.  */
 #define FALLTHROUGH _GL_ATTRIBUTE_FALLTHROUGH
 
-/* Tells that it's OK if the entity is not used.  No "is not used" warning
-   shall be produced for the entity.  */
+/* Do not warn if the entity is not used.  */
 /* Applies to:
  - function, variable,
  - struct, union, struct/union member,
@@ -59,8 +57,8 @@
in C++ also: class.  */
 #define MAYBE_UNUSED _GL_ATTRIBUTE_MAYBE_UNUSED
 
-/* Tells that it is important for the caller of a function to inspect its
-   return value.  Discarding the return value produces a warning.  */
+/* Warn if the caller does not use the return value,
+   unless the caller uses something like ignore_value.  */
 /* Applies to: function, enumeration, class.  */
 #define NODISCARD _GL_ATTRIBUTE_NODISCARD
 
@@ -71,141 +69,112 @@
https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html
These names begin with 'ATTRIBUTE_' to avoid name clashes.  */
 
-/* Used for memory allocation functions.
-   ATTRIBUTE_ALLOC_SIZE ((n)) specifies that the function returns a block
-   of memory whose size is the n-th argument (n counts from 1).
-   ATTRIBUTE_ALLOC_SIZE ((m, n)) specifies that the function returns a block
-   of memory whose size is the product of the m-th argument and the n-th
-   argument.  */
-/* Applies to: functions, pointers to functions, function types */
+/* ATTRIBUTE_ALLOC_SIZE ((N)) - The Nth argument of the function
+   is the size of the returned memory block.
+   ATTRIBUTE_ALLOC_SIZE ((M, N)) - Multiply the Mth and Nth arguments
+   to determine the size of the returned memory block.  */
+/* Applies to: function, pointer to function, function types.  */
 #define ATTRIBUTE_ALLOC_SIZE(args) _GL_ATTRIBUTE_ALLOC_SIZE (args)
 
-/* Forces the function to be inlined.  Results in an error if the compiler
-   cannot honour the inlining request.  */
-/* Applies to: functions */
+/* Always inline the function, and report an error if the compiler
+   cannot inline.  */
+/* Applies to: function.  */
 #define ATTRIBUTE_ALWAYS_INLINE _GL_ATTRIBUTE_ALWAYS_INLINE
 
-/* Used for small inline functions.
-   Specifies that the function should be omitted from stack traces.  */
-/* Applies to: functions */
+/* Omit the function from stack traces when debugging.  */
+/* Applies to: function.  */
 #define ATTRIBUTE_ARTIFICIAL _GL_ATTRIBUTE_ARTIFICIAL
 
-/* Used to denote the result of 

Re: m4: use AC_SUBST with two arguments when applicable

[Akim Demaille]

Hi Bruno,

> Le 9 mai 2020 à 17:25, Bruno Haible  a écrit :
> 
> Hi Akim,
> 
>>  AC_SUBST([GNULIB_ISWBLANK], [0])
>>  AC_SUBST([GNULIB_ISWDIGIT], [0])
>>  AC_SUBST([GNULIB_ISWXDIGIT], [0])
>>  AC_SUBST([GNULIB_WCTYPE], [0])
>>  AC_SUBST([GNULIB_ISWCTYPE], [0])
>>  AC_SUBST([GNULIB_WCTRANS], [0])
>>  AC_SUBST([GNULIB_TOWCTRANS], [0])
> 
> No, no, noo!! This would make the *.m4 code even harder to maintain.

:)

>>  gl_SUBSTS(
>>[[GNULIB_ISWBLANK], [0]],
>>[[GNULIB_ISWDIGIT], [0]],
>>[[GNULIB_ISWXDIGIT], [0]],
>>[[GNULIB_WCTYPE], [0]],
>>[[GNULIB_ISWCTYPE], [0]],
>>[[GNULIB_WCTRANS], [0]],
>>[[GNULIB_TOWCTRANS], [0]]
>>  )
> 
> Eeek! This is even worse!!!

:)

> Really, you need to consider two things when proposing new coding patterns:
> 
> 1) The semantics.
> 2) The way a programmer works with the code on a daily basis.
> 
> 1) Let's take an example: HAVE_OPENDIR. See how it's referenced in the *.m4
> files:
> 
> $ grep -rw HAVE_OPENDIR m4 modules
> m4/opendir.m4:HAVE_OPENDIR=0
> m4/opendir.m4:  if test $HAVE_OPENDIR = 1; then
> m4/opendir.m4:  case $host_os,$HAVE_OPENDIR in
> m4/dirent_h.m4:  HAVE_OPENDIR=1;   AC_SUBST([HAVE_OPENDIR])
> modules/opendir:filename[test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:unistd  [test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:closedir[test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:dirfd   [test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:if test $HAVE_OPENDIR = 0 || test $REPLACE_OPENDIR = 1; then
> modules/dirent:   -e 's/@''HAVE_OPENDIR''@/$(HAVE_OPENDIR)/g' \
> 
> What you see is:
>  - 2 assignments, at different places (even in different files),
>  - 2 references as shell variables in *.m4 files,
>  - more references as shell variables in the modules description,
>  - 1 AC_SUBST, so it becomes usable as @HAVE_OPENDIR@ in *.h files.
> 
> What you propose is
>  - arbitrary, because it selects one out of several assignments, to
>combine them with AC_SUBST,
>  - counter-maintainable, because it hides the fact that one of these
>is a variable assignment. When I have a shell variable, I want to
>have *all* assignments done through the VAR=... syntax and *all*
>references through $VAR syntax.

If what your pattern of work is grep foo=, then we could have

 gl_SUBSTS(
   GNULIB_ISWBLANK=0
   GNULIB_ISWDIGIT=0
   GNULIB_ISWXDIGIT=0
   GNULIB_WCTYPE=0
   GNULIB_ISWCTYPE=0
   GNULIB_WCTRANS=0
   GNULIB_TOWCTRANS=0
 )

But the current solution where you repeat things is not something I like.  
You're the boss, granted, your taste matters more than mine, granted.  But I 
dislike these repetitive and tedious lines.

> 2) I work with this code on a daily basis. Often I need to locate all
>   assignments. A 'grep -rw HAVE_OPENDIR=' does the job. What you
>   propose, would force me to search for
> 1. the 'HAVE_OPENDIR=' pattern,
> 2. the 'AC_SUBST([HAVE_OPENDIR]' pattern,
>   thus effectively doubling the time I need to make such a search.
> 
> And finally, your proposal with gl_SUBSTS groups things together that
> are semantically unrelated. There is no relation between GNULIB_ISWBLANK
> and GNULIB_WCTYPE. Therefore it is wrong to group them together.

The grouping is already there, just with more boilerplate.


> Most probably, you wanted to reduce the complexity of the *.m4 code by
> making it smaller?
> 
> A laudible goal, but unfortunately you have a totally wrong complexity
> measure.

Let's say that I would not put it this way...  But I agree we disagree.

> The complexity comes from the fact that there are different
> assignment in different files. But you can't reduce that complexity
> (or at least, I see no way of doing that).
> 
> --- --- --- ---
> 
> IF you want to reduce complexity in relation to AC_SUBST, then add
> a variant AC_SUBST_CONST in such a way that
>  - AC_SUBST_CONST(VAR) registers a substitution of VAR, like AC_SUBST
>does,
>  - AC_SUBST_CONST(VAR) asserts that VAR has already its value at this
>point,
>  - assignments to VAR that occur later are forbidden (like with 'const'
>variables in C).
> 
> This would reduce complexity, because it guarantees that the value is
> already known, which - by the way AC_REQUIRE works - means that no other
> file can modify the value.
> 
> For the maintainer, in those cases where AC_SUBST_CONST is applicable
> (e.g. HAVE_GLOB_H), it would greatly reduce the scope in which I have to
> look for assignments to the variable.
> 
> That's how you reduce complexity: Take away irrelevant freedoms, and
> make things more "functional" or "declarative" instead of "procedural".

I'm looking for means to avoid repetitions, and boilerplate.  This does
not.

Checking that the assigned value is "const" is nice though.  Yet I would
still prefer AC_SUBST_CONST([FOO=1]) than all these error prone repetitions.

attribute: add comments

[Bruno Haible]

Hi Paul,

I can't really comfortably use a set of macro, for which I need to look
up a C in-progress standard, a C++ standard, and the GCC documentation.

So, I've added documentation, that answers the questions:
  - Where should the attribute be placed?
  - On which entities is it supported?
  - What is its use-case?
  - What is the argument? Single parentheses or nested parentheses?

The text I wrote is meant be easier to understand upon first use, but
still as correct (or nearly as correct) as the corresponding GCC documentation.

Btw, why is there 'ATTRIBUTE_DEPRECATED', when there is also 'DEPRECATED',
defined in the same file and with the same definition?


2020-05-09  Bruno Haible  

attribute: Add comments.
* lib/attribute.h: Document each macro.

diff --git a/lib/attribute.h b/lib/attribute.h
index a4e12df..22416a8 100644
--- a/lib/attribute.h
+++ b/lib/attribute.h
@@ -20,39 +20,193 @@
 /* Provide public ATTRIBUTE_* names for the private _GL_ATTRIBUTE_*
macros used within Gnulib.  */
 
+/* These attributes can be placed in two ways:
+ - At the start of a declaration (i.e. even before storage-class
+   specifiers!); then they apply to all entities that are declared
+   by the declaration.
+ - Immediately after the name of an entity being declared by the
+   declaration; then they apply to that entity only.  */
+
 #ifndef _GL_ATTRIBUTE_H
 #define _GL_ATTRIBUTE_H
 
+
 /* C2X standard attributes have macro names that do not begin with
'ATTRIBUTE_'.  */
+
+/* Tells that the entity is deprecated.  A use of the entity produces a
+   warning.  */
+/* Applies to:
+ - function, variable,
+ - struct, union, struct/union member,
+ - enumeration, enumeration item,
+ - typedef,
+   in C++ also: namespace, class, template specialization.  */
 #define DEPRECATED _GL_ATTRIBUTE_DEPRECATED
+
+/* Tells that it's intentional that the control flow can go on to the
+   immediately following 'case' or 'default' label.  */
+/* Applies to: Empty statement (;), inside a 'switch' statement.  */
 #define FALLTHROUGH _GL_ATTRIBUTE_FALLTHROUGH
+
+/* Tells that it's OK if the entity is not used.  No "is not used" warning
+   shall be produced for the entity.  */
+/* Applies to:
+ - function, variable,
+ - struct, union, struct/union member,
+ - enumeration, enumeration item,
+ - typedef,
+   in C++ also: class.  */
 #define MAYBE_UNUSED _GL_ATTRIBUTE_MAYBE_UNUSED
+
+/* Tells that it is important for the caller of a function to inspect its
+   return value.  Discarding the return value produces a warning.  */
+/* Applies to: function, enumeration, class.  */
 #define NODISCARD _GL_ATTRIBUTE_NODISCARD
 
+
 /* Selected GCC attributes; see:
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
+   https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html
+   https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html
These names begin with 'ATTRIBUTE_' to avoid name clashes.  */
+
+/* Used for memory allocation functions.
+   ATTRIBUTE_ALLOC_SIZE ((n)) specifies that the function returns a block
+   of memory whose size is the n-th argument (n counts from 1).
+   ATTRIBUTE_ALLOC_SIZE ((m, n)) specifies that the function returns a block
+   of memory whose size is the product of the m-th argument and the n-th
+   argument.  */
+/* Applies to: functions, pointers to functions, function types */
 #define ATTRIBUTE_ALLOC_SIZE(args) _GL_ATTRIBUTE_ALLOC_SIZE (args)
+
+/* Forces the function to be inlined.  Results in an error if the compiler
+   cannot honour the inlining request.  */
+/* Applies to: functions */
 #define ATTRIBUTE_ALWAYS_INLINE _GL_ATTRIBUTE_ALWAYS_INLINE
+
+/* Used for small inline functions.
+   Specifies that the function should be omitted from stack traces.  */
+/* Applies to: functions */
 #define ATTRIBUTE_ARTIFICIAL _GL_ATTRIBUTE_ARTIFICIAL
+
+/* Used to denote the result of manual profiling.
+   Specifies that the function is unlikely to be executed.  */
+/* Applies to: functions */
 #define ATTRIBUTE_COLD _GL_ATTRIBUTE_COLD
+
+/* Used on functions that don't inspect memory and don't do side effects
+   and don't invoke random behaviour.
+   Specifies that the compiler is allowed to replace multiple invocations
+   of the function with the same arguments with a single invocation.  */
+/* Applies to: functions */
 #define ATTRIBUTE_CONST _GL_ATTRIBUTE_CONST
+
+/* Same as DEPRECATED, above.  */
 #define ATTRIBUTE_DEPRECATED _GL_ATTRIBUTE_DEPRECATED
+
+/* Used on functions that should not be called.
+   ATTRIBUTE_ERROR (MSG) specifies that the compiler is allowed to produce
+   an error when an invocation of the function cannot be optimized away.
+   The error message will include MSG.  */
+/* Applies to: functions */
 #define ATTRIBUTE_ERROR(msg) _GL_ATTRIBUTE_ERROR (msg)
+
+/* Used on functions or variables that shall persist across the
+   '-fwhole-program' optimization.  */
+/* Applies to: functions */
 

Re: m4: use AC_SUBST with two arguments when applicable

[Bruno Haible]

Hi Akim,

>   AC_SUBST([GNULIB_ISWBLANK], [0])
>   AC_SUBST([GNULIB_ISWDIGIT], [0])
>   AC_SUBST([GNULIB_ISWXDIGIT], [0])
>   AC_SUBST([GNULIB_WCTYPE], [0])
>   AC_SUBST([GNULIB_ISWCTYPE], [0])
>   AC_SUBST([GNULIB_WCTRANS], [0])
>   AC_SUBST([GNULIB_TOWCTRANS], [0])

No, no, noo!! This would make the *.m4 code even harder to maintain.

>   gl_SUBSTS(
> [[GNULIB_ISWBLANK], [0]],
> [[GNULIB_ISWDIGIT], [0]],
> [[GNULIB_ISWXDIGIT], [0]],
> [[GNULIB_WCTYPE], [0]],
> [[GNULIB_ISWCTYPE], [0]],
> [[GNULIB_WCTRANS], [0]],
> [[GNULIB_TOWCTRANS], [0]]
>   )

Eeek! This is even worse!!!

Really, you need to consider two things when proposing new coding patterns:

1) The semantics.
2) The way a programmer works with the code on a daily basis.

1) Let's take an example: HAVE_OPENDIR. See how it's referenced in the *.m4
files:

$ grep -rw HAVE_OPENDIR m4 modules
m4/opendir.m4:HAVE_OPENDIR=0
m4/opendir.m4:  if test $HAVE_OPENDIR = 1; then
m4/opendir.m4:  case $host_os,$HAVE_OPENDIR in
m4/dirent_h.m4:  HAVE_OPENDIR=1;   AC_SUBST([HAVE_OPENDIR])
modules/opendir:filename[test $HAVE_OPENDIR = 0 || test 
$REPLACE_OPENDIR = 1]
modules/opendir:unistd  [test $HAVE_OPENDIR = 0 || test 
$REPLACE_OPENDIR = 1]
modules/opendir:closedir[test $HAVE_OPENDIR = 0 || test 
$REPLACE_OPENDIR = 1]
modules/opendir:dirfd   [test $HAVE_OPENDIR = 0 || test 
$REPLACE_OPENDIR = 1]
modules/opendir:if test $HAVE_OPENDIR = 0 || test $REPLACE_OPENDIR = 1; then
modules/dirent:   -e 's/@''HAVE_OPENDIR''@/$(HAVE_OPENDIR)/g' \

What you see is:
  - 2 assignments, at different places (even in different files),
  - 2 references as shell variables in *.m4 files,
  - more references as shell variables in the modules description,
  - 1 AC_SUBST, so it becomes usable as @HAVE_OPENDIR@ in *.h files.

What you propose is
  - arbitrary, because it selects one out of several assignments, to
combine them with AC_SUBST,
  - counter-maintainable, because it hides the fact that one of these
is a variable assignment. When I have a shell variable, I want to
have *all* assignments done through the VAR=... syntax and *all*
references through $VAR syntax.

2) I work with this code on a daily basis. Often I need to locate all
   assignments. A 'grep -rw HAVE_OPENDIR=' does the job. What you
   propose, would force me to search for
 1. the 'HAVE_OPENDIR=' pattern,
 2. the 'AC_SUBST([HAVE_OPENDIR]' pattern,
   thus effectively doubling the time I need to make such a search.

And finally, your proposal with gl_SUBSTS groups things together that
are semantically unrelated. There is no relation between GNULIB_ISWBLANK
and GNULIB_WCTYPE. Therefore it is wrong to group them together.

 --- --- --- ---

Most probably, you wanted to reduce the complexity of the *.m4 code by
making it smaller?

A laudible goal, but unfortunately you have a totally wrong complexity
measure. The complexity comes from the fact that there are different
assignment in different files. But you can't reduce that complexity
(or at least, I see no way of doing that).

 --- --- --- ---

IF you want to reduce complexity in relation to AC_SUBST, then add
a variant AC_SUBST_CONST in such a way that
  - AC_SUBST_CONST(VAR) registers a substitution of VAR, like AC_SUBST
does,
  - AC_SUBST_CONST(VAR) asserts that VAR has already its value at this
point,
  - assignments to VAR that occur later are forbidden (like with 'const'
variables in C).

This would reduce complexity, because it guarantees that the value is
already known, which - by the way AC_REQUIRE works - means that no other
file can modify the value.

For the maintainer, in those cases where AC_SUBST_CONST is applicable
(e.g. HAVE_GLOB_H), it would greatly reduce the scope in which I have to
look for assignments to the variable.

That's how you reduce complexity: Take away irrelevant freedoms, and
make things more "functional" or "declarative" instead of "procedural".

Bruno

Re: The attribute module

[Bruno Haible]

Hi Akim,

> So there remains this one.  Ok to install?

Yes. Looks good.

Bruno

m4: use AC_SUBST with two arguments when applicable

[Akim Demaille]

Actually, given the number of times this pattern is used, I would personally 
introduce gl_SUBSTS, a variadic version of AC_SUBST, to avoid all this 
repetition, say something like

  gl_SUBSTS(
[[GNULIB_ISWBLANK], [0]],
[[GNULIB_ISWDIGIT], [0]],
[[GNULIB_ISWXDIGIT], [0]],
[[GNULIB_WCTYPE], [0]],
[[GNULIB_ISWCTYPE], [0]],
[[GNULIB_WCTRANS], [0]],
[[GNULIB_TOWCTRANS], [0]]
  )

instead of

  AC_SUBST([GNULIB_ISWBLANK], [0])
  AC_SUBST([GNULIB_ISWDIGIT], [0])
  AC_SUBST([GNULIB_ISWXDIGIT], [0])
  AC_SUBST([GNULIB_WCTYPE], [0])
  AC_SUBST([GNULIB_ISWCTYPE], [0])
  AC_SUBST([GNULIB_WCTRANS], [0])
  AC_SUBST([GNULIB_TOWCTRANS], [0])

I can take care of that if there is agreement.

Ok to install?

commit 295dc196c4f5e186a7a435fa4c7535943e815132
Author: Akim Demaille 
Date:   Sat May 9 14:08:12 2020 +0200

m4: use AC_SUBST with two arguments when applicable.

* m4/arpa_inet_h.m4, m4/dirent_h.m4, m4/fcntl_h.m4,
* m4/fnmatch_h.m4, m4/iconv_h.m4, m4/inttypes.m4, m4/langinfo_h.m4,
* m4/locale_h.m4, m4/math_h.m4, m4/monetary_h.m4, m4/netdb_h.m4,
* m4/poll_h.m4, m4/pthread_h.m4, m4/pty_h.m4, m4/sched_h.m4,
* m4/search_h.m4, m4/signal_h.m4, m4/spawn_h.m4, m4/stddef_h.m4,
* m4/stdio_h.m4, m4/stdlib_h.m4, m4/string_h.m4, m4/sys_file_h.m4,
* m4/sys_ioctl_h.m4, m4/sys_resource_h.m4, m4/sys_select_h.m4,
* m4/sys_socket_h.m4, m4/sys_stat_h.m4, m4/sys_time_h.m4,
* m4/sys_utsname_h.m4, m4/termios_h.m4, m4/threads.m4, m4/time_h.m4,
* m4/uchar.m4, m4/unistd_h.m4, m4/utime_h.m4, m4/wchar_h.m4,
* m4/wctype_h.m4:
Use AC_SUBST([FOO], [BAR]) instead of FOO=BAR; AC_SUBST([FOO]).

diff --git a/ChangeLog b/ChangeLog
index bc99924b1..4cc9d30cd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2020-05-09  Akim Demaille  
+
+   m4: use AC_SUBST with two arguments when applicable.
+   * m4/arpa_inet_h.m4, m4/dirent_h.m4, m4/fcntl_h.m4,
+   * m4/fnmatch_h.m4, m4/iconv_h.m4, m4/inttypes.m4, m4/langinfo_h.m4,
+   * m4/locale_h.m4, m4/math_h.m4, m4/monetary_h.m4, m4/netdb_h.m4,
+   * m4/poll_h.m4, m4/pthread_h.m4, m4/pty_h.m4, m4/sched_h.m4,
+   * m4/search_h.m4, m4/signal_h.m4, m4/spawn_h.m4, m4/stddef_h.m4,
+   * m4/stdio_h.m4, m4/stdlib_h.m4, m4/string_h.m4, m4/sys_file_h.m4,
+   * m4/sys_ioctl_h.m4, m4/sys_resource_h.m4, m4/sys_select_h.m4,
+   * m4/sys_socket_h.m4, m4/sys_stat_h.m4, m4/sys_time_h.m4,
+   * m4/sys_utsname_h.m4, m4/termios_h.m4, m4/threads.m4, m4/time_h.m4,
+   * m4/uchar.m4, m4/unistd_h.m4, m4/utime_h.m4, m4/wchar_h.m4,
+   * m4/wctype_h.m4:
+   Use AC_SUBST([FOO], [BAR]) instead of FOO=BAR; AC_SUBST([FOO]).
+
 2020-05-09  Akim Demaille  
 
bitset: use the attribute module
diff --git a/m4/arpa_inet_h.m4 b/m4/arpa_inet_h.m4
index b39449494..3f0562b05 100644
--- a/m4/arpa_inet_h.m4
+++ b/m4/arpa_inet_h.m4
@@ -1,4 +1,4 @@
-# arpa_inet_h.m4 serial 14
+# arpa_inet_h.m4 serial 15
 dnl Copyright (C) 2006, 2008-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -49,11 +49,11 @@ AC_DEFUN([gl_ARPA_INET_MODULE_INDICATOR],
 
 AC_DEFUN([gl_ARPA_INET_H_DEFAULTS],
 [
-  GNULIB_INET_NTOP=0; AC_SUBST([GNULIB_INET_NTOP])
-  GNULIB_INET_PTON=0; AC_SUBST([GNULIB_INET_PTON])
+  AC_SUBST([GNULIB_INET_NTOP], [0])
+  AC_SUBST([GNULIB_INET_PTON], [0])
   dnl Assume proper GNU behavior unless another module says otherwise.
-  HAVE_DECL_INET_NTOP=1;  AC_SUBST([HAVE_DECL_INET_NTOP])
-  HAVE_DECL_INET_PTON=1;  AC_SUBST([HAVE_DECL_INET_PTON])
-  REPLACE_INET_NTOP=0;AC_SUBST([REPLACE_INET_NTOP])
-  REPLACE_INET_PTON=0;AC_SUBST([REPLACE_INET_PTON])
+  AC_SUBST([HAVE_DECL_INET_NTOP], [1])
+  AC_SUBST([HAVE_DECL_INET_PTON], [1])
+  AC_SUBST([REPLACE_INET_NTOP],   [0])
+  AC_SUBST([REPLACE_INET_PTON],   [0])
 ])
diff --git a/m4/dirent_h.m4 b/m4/dirent_h.m4
index 8bef6a0ce..93130618f 100644
--- a/m4/dirent_h.m4
+++ b/m4/dirent_h.m4
@@ -1,4 +1,4 @@
-# dirent_h.m4 serial 16
+# dirent_h.m4 serial 17
 dnl Copyright (C) 2008-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -39,26 +39,26 @@ AC_DEFUN([gl_DIRENT_MODULE_INDICATOR],
 AC_DEFUN([gl_DIRENT_H_DEFAULTS],
 [
   AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) dnl for REPLACE_FCHDIR
-  GNULIB_OPENDIR=0; AC_SUBST([GNULIB_OPENDIR])
-  GNULIB_READDIR=0; AC_SUBST([GNULIB_READDIR])
-  GNULIB_REWINDDIR=0;   AC_SUBST([GNULIB_REWINDDIR])
-  GNULIB_CLOSEDIR=0;AC_SUBST([GNULIB_CLOSEDIR])
-  GNULIB_DIRFD=0;   AC_SUBST([GNULIB_DIRFD])
-  GNULIB_FDOPENDIR=0;   AC_SUBST([GNULIB_FDOPENDIR])
-  GNULIB_SCANDIR=0; AC_SUBST([GNULIB_SCANDIR])
-  GNULIB_ALPHASORT=0;   AC_SUBST([GNULIB_ALPHASORT])
+  AC_SUBST([GNULIB_OPENDIR],   [0])
+  AC_SUBST([GNULIB_READDIR],   [0])
+  AC_SUBST([GNULIB_REWINDDIR], [0])
+  AC_SUBST([GNULIB_CLOSEDIR],  

Re: The attribute module

[Akim Demaille]

> Le 9 mai 2020 à 13:24, Bruno Haible  a écrit :
> 
> Hi Akim,
> 
>>> In file included from ../lib/bitsetv.c:21:
>>> In file included from ../lib/bitsetv.h:24:
>>> In file included from ../lib/bitset.h:31:
>>> In file included from ../lib/bitset/base.h:28:
>>> ../lib/xalloc.h:43:10: warning: '_GL_ATTRIBUTE_ALLOC_SIZE' macro redefined 
>>> [-Wmacro-redefined]
>>> # define _GL_ATTRIBUTE_ALLOC_SIZE(args)
>>> ^
>>> ./lib/config.h:1657:10: note: previous definition is here
>>> # define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ 
>>> args))
>>> ^
> 
> These two patches should fix these warnings.

Ah, I had the second one done too :)

So there remains this one.  Ok to install?



0002-bitset-use-the-attribute-module.patch
Description: Binary data

Re: The attribute module

[Akim Demaille]

Hi Bruno,

> Le 9 mai 2020 à 12:37, Bruno Haible  a écrit :
> 
> Hi Akim,
> 
> Also, I avoid to use _GL_ATTRIBUTE_FORMAT and ATTRIBUTE_FORMAT, because of a
> tricky override of 'printf' by '__printf__', done in gnulib and libintl.

Then, shouldn't gnulib expose the _GL_ATTRIBUTE_FORMAT_(PRINTF|SCANF)(_SYSTEM)? 
macros as defined in stdio.in.h?

Re: c-stack.c and DEBUG: missing import

[Bruno Haible]

Hi Marc,

> Please add
> 
> #ifdef DEBUG
> # include 
> #endif
> 
> at the beginning of c-stack.c.
> 
> When the DEBUG flag is enabled, c-stack.c uses sprintf and without the
> suggested addition gcc complains about an implicit declaration of the
> function sprintf.

Yup. Thanks for the suggestion. Done.


2020-05-09  Bruno Haible  

c-stack: Fix warning when DEBUG is enabled.
Patch suggested by Marc Nieper-Wißkirchen  in
.
* lib/c-stack.c: Include .

diff --git a/lib/c-stack.c b/lib/c-stack.c
index 4050d08..e486591 100644
--- a/lib/c-stack.c
+++ b/lib/c-stack.c
@@ -65,6 +65,10 @@ typedef struct sigaltstack stack_t;
 
 #include 
 
+#if DEBUG
+# include 
+#endif
+
 #if HAVE_LIBSIGSEGV
 # include 
 #endif

Re: The attribute module

[Bruno Haible]

Hi Akim,

> > In file included from ../lib/bitsetv.c:21:
> > In file included from ../lib/bitsetv.h:24:
> > In file included from ../lib/bitset.h:31:
> > In file included from ../lib/bitset/base.h:28:
> > ../lib/xalloc.h:43:10: warning: '_GL_ATTRIBUTE_ALLOC_SIZE' macro redefined 
> > [-Wmacro-redefined]
> > # define _GL_ATTRIBUTE_ALLOC_SIZE(args)
> >  ^
> > ./lib/config.h:1657:10: note: previous definition is here
> > # define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ 
> > args))
> >  ^

These two patches should fix these warnings.


2020-05-09  Bruno Haible  

Remove redundant definitions of _GL_ATTRIBUTE_FORMAT.
* lib/argp.h (_GL_ATTRIBUTE_FORMAT): Remove macro.
* lib/argp-fmtstream.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/c-snprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/c-vasnprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/c-vasprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/c-vsnprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/c-xvasprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/error.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/parse-datetime.y (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/vasnprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/xprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.
* lib/xvasprintf.h (_GL_ATTRIBUTE_FORMAT): Likewise.

2020-05-09  Bruno Haible  

Remove redundant definitions of _GL_ATTRIBUTE_ALLOC_SIZE.
Reported by Akim Demaille in
.
* lib/eealloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Remove macro.
* lib/pagealign_alloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Likewise.
* lib/xalloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Likewise.

>From 7dc2704acc113493518945b2f751dfb2882a698a Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sat, 9 May 2020 13:14:24 +0200
Subject: [PATCH 1/2] Remove redundant definitions of _GL_ATTRIBUTE_ALLOC_SIZE.

Reported by Akim Demaille in
.

* lib/eealloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Remove macro.
* lib/pagealign_alloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Likewise.
* lib/xalloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Likewise.
---
 ChangeLog | 9 +
 lib/eealloc.h | 7 ---
 lib/pagealign_alloc.h | 7 ---
 lib/xalloc.h  | 7 ---
 4 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7f19e4e..dff097d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2020-05-09  Bruno Haible  
 
+	Remove redundant definitions of _GL_ATTRIBUTE_ALLOC_SIZE.
+	Reported by Akim Demaille in
+	.
+	* lib/eealloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Remove macro.
+	* lib/pagealign_alloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Likewise.
+	* lib/xalloc.h (_GL_ATTRIBUTE_ALLOC_SIZE): Likewise.
+
+2020-05-09  Bruno Haible  
+
 	stdio, monetary: Don't redefine _GL_ATTRIBUTE_FORMAT.
 	* lib/stdio.in.h (_GL_ATTRIBUTE_FORMAT): Don't override the definition
 	that usually comes from m4/gnulib-common.m4.
diff --git a/lib/eealloc.h b/lib/eealloc.h
index 351cbed..328268e 100644
--- a/lib/eealloc.h
+++ b/lib/eealloc.h
@@ -39,13 +39,6 @@ _GL_INLINE_HEADER_BEGIN
 # define EEALLOC_INLINE _GL_INLINE
 #endif
 
-#if ! defined __clang__ && \
-(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3))
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
-#else
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args)
-#endif
-
 #if MALLOC_0_IS_NONNULL
 # define eemalloc malloc
 #else
diff --git a/lib/pagealign_alloc.h b/lib/pagealign_alloc.h
index 1459ec4..393eb18 100644
--- a/lib/pagealign_alloc.h
+++ b/lib/pagealign_alloc.h
@@ -20,13 +20,6 @@
 
 # include 
 
-#if ! defined __clang__ && \
-(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3))
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
-#else
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args)
-#endif
-
 /* Allocate a block of memory of SIZE bytes, aligned on a system page
boundary.
If SIZE is not a multiple of the system page size, it will be rounded up
diff --git a/lib/xalloc.h b/lib/xalloc.h
index 19c64ac..24273ff 100644
--- a/lib/xalloc.h
+++ b/lib/xalloc.h
@@ -36,13 +36,6 @@ extern "C" {
 #endif
 
 
-#if ! defined __clang__ && \
-(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3))
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
-#else
-# define _GL_ATTRIBUTE_ALLOC_SIZE(args)
-#endif
-
 /* This function is always triggered when memory is exhausted.
It must be defined by the application, either explicitly
or by using gnulib's xalloc-die module.  This is the
-- 
2.7.4

>From 598afc1407a86340f948e5545f19c06afe13e059 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sat, 9 May 2020 13:22:49 +0200
Subject: [PATCH 2/2] Remove redundant 

c-stack.c and DEBUG: missing import

[Marc Nieper-Wißkirchen]

Please add

#ifdef DEBUG
# include 
#endif

at the beginning of c-stack.c.

When the DEBUG flag is enabled, c-stack.c uses sprintf and without the
suggested addition gcc complains about an implicit declaration of the
function sprintf.

Thanks,

Marc

Re: The attribute module

[Bruno Haible]

I wrote:
> - In the lib/*.in.h header files, that may (in some circumstances) be used
>   without a preceding '#include ' and that therefore are 
> designed
>   to be as standalone as possible, you can use the _GL_ATTRIBUTE_* macros
>   if you have added a fallback definition.

The fallback definition was not guarded so far. It becomes relevant now
that m4/gnulib-common.m4 contains the more elaborate definition.


2020-05-09  Bruno Haible  

stdio, monetary: Don't redefine _GL_ATTRIBUTE_FORMAT.
* lib/stdio.in.h (_GL_ATTRIBUTE_FORMAT): Don't override the definition
that usually comes from m4/gnulib-common.m4.
* lib/monetary.in.h (_GL_ATTRIBUTE_FORMAT): Likewise.

2020-05-09  Bruno Haible  

dirent, stdlib, wchar, string: Don't redefine _GL_ATTRIBUTE_PURE.
* lib/dirent.in.h (_GL_ATTRIBUTE_PURE): Don't override the definition
that usually comes from m4/gnulib-common.m4.
* lib/stdlib.in.h (_GL_ATTRIBUTE_PURE): Likewise.
* lib/string.in.h (_GL_ATTRIBUTE_PURE): Likewise.
* lib/wchar.in.h (_GL_ATTRIBUTE_PURE): Likewise.

>From dae13c0c738762e9522b100868364bd08a080321 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sat, 9 May 2020 12:50:57 +0200
Subject: [PATCH 1/2] dirent, stdlib, wchar, string: Don't redefine
 _GL_ATTRIBUTE_PURE.

* lib/dirent.in.h (_GL_ATTRIBUTE_PURE): Don't override the definition
that usually comes from m4/gnulib-common.m4.
* lib/stdlib.in.h (_GL_ATTRIBUTE_PURE): Likewise.
* lib/string.in.h (_GL_ATTRIBUTE_PURE): Likewise.
* lib/wchar.in.h (_GL_ATTRIBUTE_PURE): Likewise.
---
 ChangeLog   |  9 +
 lib/dirent.in.h | 10 ++
 lib/stdlib.in.h | 10 ++
 lib/string.in.h | 10 ++
 lib/wchar.in.h  | 10 ++
 5 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0d9ab54..065628f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2020-05-09  Bruno Haible  
 
+	dirent, stdlib, wchar, string: Don't redefine _GL_ATTRIBUTE_PURE.
+	* lib/dirent.in.h (_GL_ATTRIBUTE_PURE): Don't override the definition
+	that usually comes from m4/gnulib-common.m4.
+	* lib/stdlib.in.h (_GL_ATTRIBUTE_PURE): Likewise.
+	* lib/string.in.h (_GL_ATTRIBUTE_PURE): Likewise.
+	* lib/wchar.in.h (_GL_ATTRIBUTE_PURE): Likewise.
+
+2020-05-09  Bruno Haible  
+
 	uchar: Work around incorrect char16_t, char32_t types on Haiku 2020.
 	* lib/uchar.in.h (char16_t): Define as macro if
 	GNULIB_OVERRIDES_CHAR16_T.
diff --git a/lib/dirent.in.h b/lib/dirent.in.h
index f7c2681..6fa44f0 100644
--- a/lib/dirent.in.h
+++ b/lib/dirent.in.h
@@ -57,10 +57,12 @@ typedef struct gl_directory DIR;
 
 /* The __attribute__ feature is available in gcc versions 2.5 and later.
The attribute __pure__ was added in gcc 2.96.  */
-#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
-# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
-#else
-# define _GL_ATTRIBUTE_PURE /* empty */
+#ifndef _GL_ATTRIBUTE_PURE
+# if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
+#  define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
+# else
+#  define _GL_ATTRIBUTE_PURE /* empty */
+# endif
 #endif
 
 /* The definitions of _GL_FUNCDECL_RPL etc. are copied here.  */
diff --git a/lib/stdlib.in.h b/lib/stdlib.in.h
index ec5f124..59f9e6c 100644
--- a/lib/stdlib.in.h
+++ b/lib/stdlib.in.h
@@ -101,10 +101,12 @@ struct random_data
 
 /* The __attribute__ feature is available in gcc versions 2.5 and later.
The attribute __pure__ was added in gcc 2.96.  */
-#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
-# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
-#else
-# define _GL_ATTRIBUTE_PURE /* empty */
+#ifndef _GL_ATTRIBUTE_PURE
+# if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
+#  define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
+# else
+#  define _GL_ATTRIBUTE_PURE /* empty */
+# endif
 #endif
 
 /* The definition of _Noreturn is copied here.  */
diff --git a/lib/string.in.h b/lib/string.in.h
index 87155ca..d601450 100644
--- a/lib/string.in.h
+++ b/lib/string.in.h
@@ -54,10 +54,12 @@
 
 /* The __attribute__ feature is available in gcc versions 2.5 and later.
The attribute __pure__ was added in gcc 2.96.  */
-#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
-# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
-#else
-# define _GL_ATTRIBUTE_PURE /* empty */
+#ifndef _GL_ATTRIBUTE_PURE
+# if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
+#  define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
+# else
+#  define _GL_ATTRIBUTE_PURE /* empty */
+# endif
 #endif
 
 /* NetBSD 5.0 declares strsignal in , not in .  */
diff --git a/lib/wchar.in.h b/lib/wchar.in.h
index 040065a..e1fa92f 100644
--- a/lib/wchar.in.h
+++ b/lib/wchar.in.h
@@ -94,10 +94,12 @@
 
 /* The __attribute__ feature is available in gcc versions 2.5 and later.
The attribute __pure__ was added in gcc 2.96.  */
-#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ 

Re: The attribute module

[Bruno Haible]

Hi Akim,

> Bison started to use the new attribute module, but it generates conflicts 
> with other modules, because one definition comes from config.h, and another 
> from the other module's file.  For instance:
> 
> > In file included from ../lib/bitsetv.c:21:
> > In file included from ../lib/bitsetv.h:24:
> > In file included from ../lib/bitset.h:31:
> > In file included from ../lib/bitset/base.h:28:
> > ../lib/xalloc.h:43:10: warning: '_GL_ATTRIBUTE_ALLOC_SIZE' macro redefined 
> > [-Wmacro-redefined]
> > # define _GL_ATTRIBUTE_ALLOC_SIZE(args)
> >  ^
> > ./lib/config.h:1657:10: note: previous definition is here
> > # define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ 
> > args))
> >  ^
> 
> I can provide a patch to address this, but I don't know what's the guideline 
> here.

The guideline is:

  * The _GL_ATTRIBUTE_* macros are defined in m4/gnulib-common.m4, from where
the definitions are propagated into config.h.
- In application code, you MAY use these macros, at your own risk - since
  they are not documented so far.
- In the lib/*.in.h header files, that may (in some circumstances) be used
  without a preceding '#include ' and that therefore are designed
  to be as standalone as possible, you can use the _GL_ATTRIBUTE_* macros
  if you have added a fallback definition.
- In all the rest of gnulib, you can use these macros directly and
  unconditionally.

  * The ATTRIBUTE_* macros are defined through attribute.h, but rely on
m4/gnulib-common.m4.
- In application code, this is the preferred API.
- In the lib/*.in.h header files, you should not use these macros, not
  even with a fallback definition, since symbols that don't start with _GL_
  could too easily collide with application code's symbols.
- In all the rest of gnulib, you can use these macros, after having added
  '#include "attribute.h"' and a module dependency towards 'attribute'.

Also, I avoid to use _GL_ATTRIBUTE_FORMAT and ATTRIBUTE_FORMAT, because of a
tricky override of 'printf' by '__printf__', done in gnulib and libintl.

Also, in code shared with glibc, e.g. the regex module, we haven't started
to use either. Recall that there are periodic manual sync-ups between glibc
and gnulib.

> Also, what's the rule inside gnulib to decide whether to use _GL_ATTRIBUTE_FOO
> rather than using attribute.h and ATTRIBUTE_FOO?

In most of gnulib, e.g. the 'bitset' module, it's a matter of taste, I'd say.

Bruno

uchar: Work around incorrect char16_t, char32_t types on Haiku 2020

[Bruno Haible]

On a recent Haiku/x86_64, a gnulib testdir fails to compile:

In file included from ../../gltests/test-uchar.c:23:
../../gltests/../gllib/verify.h:216:41: error: static assertion failed: "verify 
((char16_t)(-1) >= 0)"
 # define _GL_VERIFY(R, DIAGNOSTIC, ...) _Static_assert (R, DIAGNOSTIC)
 ^~
../../gltests/../gllib/verify.h:276:20: note: in expansion of macro '_GL_VERIFY'
 # define verify(R) _GL_VERIFY (R, "verify (" #R ")", -)
^~
../../gltests/test-uchar.c:32:1: note: in expansion of macro 'verify'
 verify ((char16_t)(-1) >= 0);
 ^~
../../gltests/../gllib/verify.h:216:41: error: static assertion failed: "verify 
((char32_t)(-1) >= 0)"
 # define _GL_VERIFY(R, DIAGNOSTIC, ...) _Static_assert (R, DIAGNOSTIC)
 ^~
../../gltests/../gllib/verify.h:276:20: note: in expansion of macro '_GL_VERIFY'
 # define verify(R) _GL_VERIFY (R, "verify (" #R ")", -)
^~
../../gltests/test-uchar.c:34:1: note: in expansion of macro 'verify'
 verify ((char32_t)(-1) >= 0);
 ^~

The reason is that the char16_t and char32_t types are incorrectly defined.
Reported at . But since the bug exists
already for two years and is fatal, let me add a workaround.


2020-05-09  Bruno Haible  

uchar: Work around incorrect char16_t, char32_t types on Haiku 2020.
* lib/uchar.in.h (char16_t): Define as macro if
GNULIB_OVERRIDES_CHAR16_T.
(char32_t): Define as macro if GNULIB_OVERRIDES_CHAR32_T.
* m4/uchar.m4 (gl_TYPE_CHAR16_T, gl_TYPE_CHAR32_T): New macros.
(gl_UCHAR_H): Invoke them.
(gl_UCHAR_H_DEFAULTS): Initialize GNULIB_OVERRIDES_CHAR16_T,
GNULIB_OVERRIDES_CHAR32_T.
* m4/mbrtoc32.m4 (gl_FUNC_MBRTOC32, gl_MBRTOC32_SANITYCHECK): Require
gl_TYPE_CHAR32_T and test GNULIB_OVERRIDES_CHAR32_T.
* modules/uchar (Makefile.am): Substitute GNULIB_OVERRIDES_CHAR16_T,
GNULIB_OVERRIDES_CHAR32_T.

diff --git a/lib/uchar.in.h b/lib/uchar.in.h
index 9c76cc9..b77116d 100644
--- a/lib/uchar.in.h
+++ b/lib/uchar.in.h
@@ -47,11 +47,25 @@
on which __STDC_UTF_16__ is defined.)  */
 typedef uint_least16_t char16_t;
 
+#elif @GNULIB_OVERRIDES_CHAR16_T@
+
+typedef uint_least16_t gl_char16_t;
+# define char16_t gl_char16_t
+
+#endif
+
+#if !@HAVE_UCHAR_H@
+
 /* A 32-bit variant of wchar_t.
Note: This type does *NOT* denote UTF-32 code points.  (Only on platforms
on which __STDC_UTF_32__ is defined.)  */
 typedef uint_least32_t char32_t;
 
+#elif @GNULIB_OVERRIDES_CHAR32_T@
+
+typedef uint_least32_t gl_char32_t;
+# define char32_t gl_char32_t
+
 #endif
 
 /* Define if a 'char32_t' can hold more characters than a 'wchar_t'.  */
diff --git a/m4/mbrtoc32.m4 b/m4/mbrtoc32.m4
index 991bbc8..5b5e938 100644
--- a/m4/mbrtoc32.m4
+++ b/m4/mbrtoc32.m4
@@ -1,4 +1,4 @@
-# mbrtoc32.m4 serial 4
+# mbrtoc32.m4 serial 5
 dnl Copyright (C) 2014-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -11,13 +11,14 @@ AC_DEFUN([gl_FUNC_MBRTOC32],
   AC_REQUIRE([AC_TYPE_MBSTATE_T])
   gl_MBSTATE_T_BROKEN
 
+  AC_REQUIRE([gl_TYPE_CHAR32_T])
   AC_REQUIRE([gl_MBRTOC32_SANITYCHECK])
 
   AC_REQUIRE([gl_CHECK_FUNC_MBRTOC32])
   if test $gl_cv_func_mbrtoc32 = no; then
 HAVE_MBRTOC32=0
   else
-if test $REPLACE_MBSTATE_T = 1; then
+if test $GNULIB_OVERRIDES_CHAR32_T = 1 || test $REPLACE_MBSTATE_T = 1; then
   REPLACE_MBRTOC32=1
 else
   gl_MBRTOC32_EMPTY_INPUT
@@ -141,11 +142,12 @@ dnl Result is HAVE_WORKING_MBRTOC32.
 AC_DEFUN([gl_MBRTOC32_SANITYCHECK],
 [
   AC_REQUIRE([AC_PROG_CC])
+  AC_REQUIRE([gl_TYPE_CHAR32_T])
   AC_REQUIRE([gl_CHECK_FUNC_MBRTOC32])
   AC_REQUIRE([gt_LOCALE_FR])
   AC_REQUIRE([gt_LOCALE_ZH_CN])
   AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
-  if test $gl_cv_func_mbrtoc32 = no; then
+  if test $GNULIB_OVERRIDES_CHAR32_T = 1 || test $gl_cv_func_mbrtoc32 = no; 
then
 HAVE_WORKING_MBRTOC32=0
   else
 AC_CACHE_CHECK([whether mbrtoc32 works as well as mbrtowc],
diff --git a/m4/uchar.m4 b/m4/uchar.m4
index 63d8b13..0875caa 100644
--- a/m4/uchar.m4
+++ b/m4/uchar.m4
@@ -1,4 +1,4 @@
-# uchar.m4 serial 13
+# uchar.m4 serial 14
 dnl Copyright (C) 2019-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -19,6 +19,9 @@ AC_DEFUN_ONCE([gl_UCHAR_H],
   fi
   AC_SUBST([HAVE_UCHAR_H])
 
+  gl_TYPE_CHAR16_T
+  gl_TYPE_CHAR32_T
+
   dnl Test whether a 'char32_t' can hold more characters than a 'wchar_t'.
   gl_STDINT_BITSIZEOF([wchar_t], [gl_STDINT_INCLUDES])
   if test $BITSIZEOF_WCHAR_T -lt 32; then
@@ -36,6 +39,51 @@ AC_DEFUN_ONCE([gl_UCHAR_H],
 ]], [c32rtomb mbrtoc32])
 ])
 
+dnl On Haiku 2020, char16_t and char32_t 

The attribute module

[Akim Demaille]

Hi all,

Bison started to use the new attribute module, but it generates conflicts with 
other modules, because one definition comes from config.h, and another from the 
other module's file.  For instance:

> In file included from ../lib/bitsetv.c:21:
> In file included from ../lib/bitsetv.h:24:
> In file included from ../lib/bitset.h:31:
> In file included from ../lib/bitset/base.h:28:
> ../lib/xalloc.h:43:10: warning: '_GL_ATTRIBUTE_ALLOC_SIZE' macro redefined 
> [-Wmacro-redefined]
> # define _GL_ATTRIBUTE_ALLOC_SIZE(args)
>  ^
> ./lib/config.h:1657:10: note: previous definition is here
> # define _GL_ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
>  ^

I can provide a patch to address this, but I don't know what's the guideline 
here.  Should (virtually) all the modules eventually depend on "attribute" and 
stop defining these macros themselves, or should their macro definitions be 
robust to the now possibly existing macros?

Also, what's the rule inside gnulib to decide whether to use _GL_ATTRIBUTE_FOO 
rather than using attribute.h and ATTRIBUTE_FOO?

Cheers!