Re: [PATCH v2] gettext.h: add parentheses around N_ expansion if supported

On Jan 8, 2015, at 11:10, Junio C Hamano wrote:

"Kyle J. McKay" [off-list ref] writes:

quoted

For now only __GNUC__ is tested which covers both gcc and clang
which should result in early detection of any adjacent N_ macros.

I didn't check the list of -W options, but if there were a way to
tell gcc to stick to the C standard in a more strict way than its
default, wouldn't this patch start causing trouble?

With this test program:

-----BEGIN TEST.C-----
#include <stdio.h>

#define msg1(x) x
#define msg2(x) (x)

static const char *const a1[] = {
	msg1("hi"),
	msg2("bye") /* line 8 */
};

static const char s1[] = msg1("hi");

static const char s2[] = msg2("bye"); /* line 13 */

int main()
{
	puts(a1[0]);
	puts(a1[1]);
	puts(s1);
	puts(s2);
	return 0;
}
-----END TEST.C-----

gcc, (but not clang) emits a warning (it still compiles just fine)  
when -pedantic is used:

   test.c:13: warning: array initialized from parenthesized string  
constant

However, none of -ansi, -Wall or -Wextra trigger that warning.

Neither does using -ansi -Wall -Wextra together cause a warning to be  
emitted (by either gcc or clang).

Note that line 8 never causes a problem (nor should it), only line 13  
is in question.

After a quick read-through of the -Wxxx options there does not appear  
to be a separate -Wxxx option to get that particular warning.

And compiling Git with -pedantic spits out a LOT of warnings (over  
7200) even before making the "(msgid)" change so I don't think there's  
an issue as apparently -pedantic is not normally used to compile Git.

Note that Git will not compile with gcc using -ansi (unless you add - 
Dinline=__inline__) and the change does not cause any new warnings to  
be emitted with -ansi (after adding the needed -Dinline=__inline__)  
since -pedantic is required for the "parenthesized string constant"  
warning.

I'm not super attached to this change, it's just that it seems to me  
that translation support for Git is a scarce resource.  I'm guessing  
that when considering the 7 complete translations (bg, ca, de, fr, sv,  
vi and zh_CN) the average number of translators per language is in the  
low single digits.  So I hate to see unnecessary translation churn,  
not when it can be so easily prevented.

-Kyle

quoted

Although the necessary #ifdef makes the header less elegant,
the benefit of avoiding propagation of a translation-marking
error to all the translation teams thus creating extra work
for them when the error is eventually detected and fixed would
seem to outweigh the minor inelegance the #ifdef introduces.

Signed-off-by: Kyle J. McKay <redacted>
---
gettext.h | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gettext.h b/gettext.h
index 7671d09d..80ec29b5 100644
--- a/gettext.h
+++ b/gettext.h

@@ -62,7 +62,19 @@ const char *Q_(const char *msgid, const char  

*plu, unsigned long n)
	return ngettext(msgid, plu, n);
}

-/* Mark msgid for translation but do not translate it. */
+/* Mark msgid for translation but do not translate it.
+ *
+ * In order to prevent accidents where two adjacent N_ macros
+ * are mistakenly used, this macro is defined with parentheses
+ * when the compiler is known to support parenthesized string
+ * literal assignments.  This guarantees a compiler error in
+ * such a case rather than a silent conjoining of the strings
+ * by the preprocessor which results in translation failures.
+ */
+#ifdef __GNUC__
+#define N_(msgid) (msgid)
+#else
#define N_(msgid) msgid
+#endif

#endif