Hi,
Ævar Arnfjörð Bjarmason wrote:
> Remove code that depend on HAVE_VARIADIC_MACROS not being set. Since
> 765dc168882 (git-compat-util: always enable variadic macros,
> 2021-01-28) we've unconditionally defined it to be true, and that
> change went out with v2.31.0. This should have given packagers enough
> time to discover whether variadic macros were an issue.
>
> It seems that they weren't, so let's update the coding guidelines and
> remove all the fallback code for the non-HAVE_VARIADIC_MACROS case.
As discussed in the rest of this thread, that's a pretty short time,
so while I would love to be able to use variadic macros
unconditionally, I think we'd need a different rationale.
That said, I want us to be ready the moment external conditions allow.
Ideally we want it to be as simple as
git grep --name-only -e HAVE_VARIADIC_MACROS |
xargs unifdef -m -DHAVE_VARIADIC_MACROS=1
plus removing the #define; is there anything we need to do in advance
to make that work well? Let's see.
[...]
> --- a/trace.h
> +++ b/trace.h
> @@ -126,66 +126,6 @@ void trace_command_performance(const char **argv);
> void trace_verbatim(struct trace_key *key, const void *buf, unsigned len);
> uint64_t trace_performance_enter(void);
>
> -#ifndef HAVE_VARIADIC_MACROS
> -
> -/**
> - * Prints a formatted message, similar to printf.
> - */
> -__attribute__((format (printf, 1, 2)))
> -void trace_printf(const char *format, ...);
This removes the documentation for these functions and doesn't add it
back on the #else side. So I think we'd want the following
preparatory patch.
Thanks,
Jonathan
-- >8 --
Subject: trace: move comments to variadic macro variant of trace functions
Nowadays compilers not having variadic macros are the exception. Move
API documentation to the declarations used in the common case.
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
trace.h | 79 +++++++++++++++++++++++++++------------------------------
1 file changed, 38 insertions(+), 41 deletions(-)
diff --git a/trace.h b/trace.h
index 0dbbad0e41..fc7eb0bc72 100644
--- a/trace.h
+++ b/trace.h
@@ -128,56 +128,22 @@ uint64_t trace_performance_enter(void);
#ifndef HAVE_VARIADIC_MACROS
-/**
- * Prints a formatted message, similar to printf.
- */
+/* Fallback implementation that does not add file:line. */
+
__attribute__((format (printf, 1, 2)))
void trace_printf(const char *format, ...);
__attribute__((format (printf, 2, 3)))
void trace_printf_key(struct trace_key *key, const char *format, ...);
-/**
- * Prints a formatted message, followed by a quoted list of arguments.
- */
__attribute__((format (printf, 2, 3)))
void trace_argv_printf(const char **argv, const char *format, ...);
-/**
- * Prints the strbuf, without additional formatting (i.e. doesn't
- * choke on `%` or even `\0`).
- */
void trace_strbuf(struct trace_key *key, const struct strbuf *data);
-/**
- * Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled.
- *
- * Example:
- * ------------
- * uint64_t t = 0;
- * for (;;) {
- * // ignore
- * t -= getnanotime();
- * // code section to measure
- * t += getnanotime();
- * // ignore
- * }
- * trace_performance(t, "frotz");
- * ------------
- */
__attribute__((format (printf, 2, 3)))
void trace_performance(uint64_t nanos, const char *format, ...);
-/**
- * Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled.
- *
- * Example:
- * ------------
- * uint64_t start = getnanotime();
- * // code section to measure
- * trace_performance_since(start, "foobar");
- * ------------
- */
__attribute__((format (printf, 2, 3)))
void trace_performance_since(uint64_t start, const char *format, ...);
@@ -186,11 +152,6 @@ void trace_performance_leave(const char *format, ...);
#else
-/*
- * Macros to add file:line - see above for C-style declarations of how these
- * should be used.
- */
-
/*
* TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The
* default is __FILE__, as it is consistent with assert(), and static function
@@ -227,8 +188,14 @@ void trace_performance_leave(const char *format, ...);
__VA_ARGS__); \
} while (0)
+/**
+ * Prints a formatted message, similar to printf.
+ */
#define trace_printf(...) trace_printf_key(&trace_default_key, __VA_ARGS__)
+/**
+ * Prints a formatted message, followed by a quoted list of arguments.
+ */
#define trace_argv_printf(argv, ...) \
do { \
if (trace_pass_fl(&trace_default_key)) \
@@ -236,12 +203,32 @@ void trace_performance_leave(const char *format, ...);
argv, __VA_ARGS__); \
} while (0)
+/**
+ * Prints the strbuf, without additional formatting (i.e. doesn't
+ * choke on `%` or even `\0`).
+ */
#define trace_strbuf(key, data) \
do { \
if (trace_pass_fl(key)) \
trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\
} while (0)
+/**
+ * Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled.
+ *
+ * Example:
+ * ------------
+ * uint64_t t = 0;
+ * for (;;) {
+ * // ignore
+ * t -= getnanotime();
+ * // code section to measure
+ * t += getnanotime();
+ * // ignore
+ * }
+ * trace_performance(t, "frotz");
+ * ------------
+ */
#define trace_performance(nanos, ...) \
do { \
if (trace_pass_fl(&trace_perf_key)) \
@@ -249,6 +236,16 @@ void trace_performance_leave(const char *format, ...);
__VA_ARGS__); \
} while (0)
+/**
+ * Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled.
+ *
+ * Example:
+ * ------------
+ * uint64_t start = getnanotime();
+ * // code section to measure
+ * trace_performance_since(start, "foobar");
+ * ------------
+ */
#define trace_performance_since(start, ...) \
do { \
if (trace_pass_fl(&trace_perf_key)) \
--
2.31.1.818.g46aad6cb9e
