Re: [PATCH 2/2] C99 support: remove non-HAVE_VARIADIC_MACROS code
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- May 21, 2021, 02:50 UTC
- Message-ID
- <YKcf95uQ1W7COSsf@google.com>
- In-Reply-To
- <patch-2.2-f12e3cad57d-20210412T105422Z-avarab@gmail.com>
Hi,
Ævar Arnfjörð Bjarmason wrote:
Show 8 quoted lines
> 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.
[...]
Show 13 quoted lines
> --- 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@gmail.com> --- 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