argmatch.h: new "_" definition may conflict with that of callers

argmatch.h: new "_" definition may conflict with that of callers

[Jim Meyering]

Hi Akim,
I tried to build grep using latest gnulib and it failed with these errors:

  CC       grep.o
In file included from grep.c:30:
../lib/argmatch.h:35: error: "_" redefined [-Werror]
   35 | # define _(Msgid)  gettext (Msgid)
      |
In file included from grep.c:28:
system.h:43: note: this is the location of the previous definition
   43 | #define _(String) gettext(String)
      |
In file included from grep.c:30:
../lib/argmatch.h:36: error: "N_" redefined [-Werror]
   36 | # define N_(Msgid) (Msgid)
      |
In file included from grep.c:28:
system.h:42: note: this is the location of the previous definition
   42 | #define N_(String) gettext_noop(String)

First, the "N_" definition appears unnecessary: it is used in neither
of argmatch.[ch].
One fix for the "_" problem is to expand it: s/\b_\b/gettext /

Care to fix it, one way or another?
Thanks,
Jim

Re: argmatch.h: new "_" definition may conflict with that of callers

[Akim Demaille]

Hi Jim!

> Le 3 juil. 2019 à 06:17, Jim Meyering <jim@meyering.net> a écrit :
> 
> Hi Akim,
> I tried to build grep using latest gnulib and it failed with these errors:
> 
>  CC       grep.o
> In file included from grep.c:30:
> ../lib/argmatch.h:35: error: "_" redefined [-Werror]
>   35 | # define _(Msgid)  gettext (Msgid)
>      |
> In file included from grep.c:28:
> system.h:43: note: this is the location of the previous definition
>   43 | #define _(String) gettext(String)
>      |
> In file included from grep.c:30:
> ../lib/argmatch.h:36: error: "N_" redefined [-Werror]
>   36 | # define N_(Msgid) (Msgid)
>      |
> In file included from grep.c:28:
> system.h:42: note: this is the location of the previous definition
>   42 | #define N_(String) gettext_noop(String)
> 
> First, the "N_" definition appears unnecessary: it is used in neither
> of argmatch.[ch].
> One fix for the "_" problem is to expand it: s/\b_\b/gettext /
> 
> Care to fix it, one way or another?

Bummer :(  Sorry about that.  I installed the following commit.

Cheers!

commit 65b3308bb67777a8e42b0d5a4550a62562790e10
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Wed Jul 3 06:35:34 2019 +0200

    argmatch: don't define _ in the header
    
    Reported by Jim Meyering.
    
    * lib/argmatch.h (N_, _): Don't define.
    Use gettext instead.
    * lib/argmatch.h (_): Define.
    * tests/test-argmatch.c (N_): Define.

diff --git a/ChangeLog b/ChangeLog
index b89fd5d9e..c3e197870 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2019-07-03  Akim Demaille  <akim@lrde.epita.fr>
+
+	argmatch: don't define _ in the header.
+	Reported by Jim Meyering.
+	* lib/argmatch.h (N_, _): Don't define.
+	Use gettext instead.
+	* lib/argmatch.h (_): Define.
+	* tests/test-argmatch.c (N_): Define.
+
 2019-07-02  Paul Eggert  <eggert@cs.ucla.edu>
 
 	verify: document ‘assume’ better
diff --git a/lib/argmatch.c b/lib/argmatch.c
index 183a0ca94..9eeb4514d 100644
--- a/lib/argmatch.c
+++ b/lib/argmatch.c
@@ -29,6 +29,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#define _(msgid) gettext (msgid)
+
 #include "error.h"
 #include "quotearg.h"
 #include "getprogname.h"
diff --git a/lib/argmatch.h b/lib/argmatch.h
index d18549942..57d131f32 100644
--- a/lib/argmatch.h
+++ b/lib/argmatch.h
@@ -32,9 +32,6 @@
 # include "quote.h"
 # include "verify.h"
 
-# define _(Msgid)  gettext (Msgid)
-# define N_(Msgid) (Msgid)
-
 # ifdef  __cplusplus
 extern "C" {
 # endif
@@ -223,7 +220,7 @@ char const *argmatch_to_argument (void const *value,
                                                                         \
     /* Try to put synonyms on the same line.  Synonyms are expected     \
        to follow each other. */                                         \
-    fputs (_("Valid arguments are:"), out);                             \
+    fputs (gettext ("Valid arguments are:"), out);                      \
     for (int i = 0; g->args[i].arg; i++)                                \
       if (i == 0                                                        \
           || memcmp (&g->args[i-1].val, &g->args[i].val, size))         \
@@ -284,7 +281,7 @@ char const *argmatch_to_argument (void const *value,
        large width. */                                                  \
     const int screen_width = getenv ("HELP2MAN") ? INT_MAX : 80;        \
     if (g->doc_pre)                                                     \
-      fprintf (out, "%s\n", _(g->doc_pre));                             \
+      fprintf (out, "%s\n", gettext (g->doc_pre));                      \
     int doc_col = argmatch_##Name##_doc_col ();                         \
     for (int i = 0; g->docs[i].arg; ++i)                                \
       {                                                                 \
@@ -321,10 +318,11 @@ char const *argmatch_to_argument (void const *value,
             fprintf (out, "\n");                                        \
             col = 0;                                                    \
           }                                                             \
-        fprintf (out, "%*s%s\n", doc_col - col, "", _(g->docs[i].doc)); \
+        fprintf (out, "%*s%s\n",                                        \
+                 doc_col - col, "", gettext (g->docs[i].doc));          \
       }                                                                 \
     if (g->doc_post)                                                    \
-      fprintf (out, "%s\n", _(g->doc_post));                            \
+      fprintf (out, "%s\n", gettext (g->doc_post));                     \
   }
 
 # ifdef  __cplusplus
diff --git a/tests/test-argmatch.c b/tests/test-argmatch.c
index 11b5819ea..7b1b38a19 100644
--- a/tests/test-argmatch.c
+++ b/tests/test-argmatch.c
@@ -25,6 +25,8 @@
 
 #include "macros.h"
 
+# define N_(Msgid) (Msgid)
+
 /* Some packages define ARGMATCH_DIE and ARGMATCH_DIE_DECL in <config.h>, and
    thus must link with a definition of that function.  Provide it here.  */
 #ifdef ARGMATCH_DIE_DECL

Re: argmatch.h: new "_" definition may conflict with that of callers

[Jim Meyering]

On Tue, Jul 2, 2019 at 9:48 PM Akim Demaille <akim.demaille@gmail.com> wrote:
> Bummer :(  Sorry about that.  I installed the following commit.
...
> +       argmatch: don't define _ in the header.
> +       Reported by Jim Meyering.
> +       * lib/argmatch.h (N_, _): Don't define.
> +       Use gettext instead.
> +       * lib/argmatch.h (_): Define.
> +       * tests/test-argmatch.c (N_): Define.

Thank you for the speedy fix!

Re: argmatch.h: new "_" definition may conflict with that of callers

[Bruno Haible]

Akim Demaille wrote:
>     * lib/argmatch.h (N_, _): Don't define.
>     Use gettext instead.

Note: That's not OK to do for all code in gnulib - since code that is
compiled into shared libraries will most often want to do
  #define _(msg) dcgettext (SOME_DOMAIN, msg)

But argmatch is unlikely to be used in a shared library, since it deals
with command-line parsing. => Fine after all.

Bruno