On Jan 8, 2015, at 11:10, Junio C Hamano wrote:
"Kyle J. McKay" <mackyle@xxxxxxxxx> writes:
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
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 <mackyle@xxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html