Re: [PATCH 1/2] git-compat-util.h: clarify comment on GCC-specific code

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 21, 2021, 02:06 UTC
Message-ID: <YKcVsfsLZqrkMjXl@google.com>
In-Reply-To: <patch-1.2-a8cc05cf56f-20210412T105422Z-avarab@gmail.com>

Hi,

Ævar Arnfjörð Bjarmason wrote:

Show 15 quoted lines

> Change a comment added in e208f9cc757 (make error()'s constant return
> value more visible, 2012-12-15) to note that the code doesn't only
> depend on variadic macros, which have been a hard dependency since
> 765dc168882 (git-compat-util: always enable variadic macros,
> 2021-01-28), but also on GCC's handling of __VA_ARGS__. The commit
> message for e208f9cc757 made this clear, but the comment it added did
> not.
>
> See also e05bed960d3 (trace: add 'file:line' to all trace output,
> 2014-07-12) for another comment about GNUC's handling of __VA_ARGS__.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  git-compat-util.h | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

Oh, subtle. In fact, I believe 9798f7e5f9 (Use __VA_ARGS__ for all of error's arguments, 2013-02-08) got rid of the gcc-ism, so we could do the following instead.

(See section 6.10.3.10 for a description of

	#define f(...) g(__VA_ARGS__)

in the C99 standard.)

Thanks, Jonathan

-- >8 --
Subject: error: use macro-based static analysis aid on non-gcc, too

In the rest of Git, we check HAVE_VARIADIC_MACROS (which is unconditionally defined to true as a way to discover platforms that do not support it) to guard code that requires variadic macro support. This variadic macro is a bit older, so it uses a __GNUC__ check instead. Switch to use of HAVE_VARIADIC_MACROS here as well, so more compilers can get the benefit of knowing at compile time that error() always returns -1.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 git-compat-util.h | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index a508dbe5a3..ca22b11760 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -483,14 +483,19 @@ void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 #include <openssl/x509v3.h>
 #endif /* NO_OPENSSL */
 
+/*
+ * This is always defined as a first step towards making the use of variadic
+ * macros unconditional. If it causes compilation problems on your platform,
+ * please report it to the Git mailing list at git@vger.kernel.org.
+ */
+#define HAVE_VARIADIC_MACROS 1
+
 /*
  * Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,
- * because some compilers may not support variadic macros. Since we're only
- * trying to help gcc, anyway, it's OK; other compilers will fall back to
- * using the function as usual.
+ * gcc with -Wuninitialized analysis. Compilers without support for variadic
+ * macros will fall back to using the function as usual.
  */
-#if defined(__GNUC__)
+#ifdef HAVE_VARIADIC_MACROS
 static inline int const_error(void)
 {
 	return -1;
@@ -1192,13 +1197,6 @@ static inline int regexec_buf(const regex_t *preg, const char *buf, size_t size,
 #endif
 #endif
 
-/*
- * This is always defined as a first step towards making the use of variadic
- * macros unconditional. If it causes compilation problems on your platform,
- * please report it to the Git mailing list at git@vger.kernel.org.
- */
-#define HAVE_VARIADIC_MACROS 1
-
 /* usage.c: only to be used for testing BUG() implementation (see test-tool) */
 extern int BUG_exit_code;

-- 
2.31.1.818.g46aad6cb9e