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

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Apr 2021 14:07:13 -0700

Jeff King <[email-masked]> writes:

>> + * We restrict this trick to gcc, though, because while we rely on the
>> + * presence of C99 variadic macros, this code also relies on the
>> + * non-standard behavior of GCC's __VA_ARGS__, allowing error() to
>> + * work even if no format specifiers are passed to error().

The last part of this comment is puzzlling.  Do we ever call error()
without any format specifier?  There may be GCC-ism behaviour around
the __VA_ARGS__ stuff, but are we relying on that GCC-ism?

>> + * Since we're only trying to help gcc, anyway, it's OK; other
>> + * compilers will fall back to using the function as usual.
>>   */
>>  #if defined(__GNUC__)
>
> I don't mind leaving this gcc-only, since as you note that's the point
> of what the code is trying to do. But wouldn't this always work because
> we know there is at least one arg (the format itself)?
>
> I.e., if we had written:
>
>   #define error(fmt, ...) (error(fmt, __VA_ARGS__), const_error())
>
> that would be a problem for:
>
>   error("foo");
>
> But because we wrote:
>
>   #define error(...) (error(__VA_ARGS__), const_error())
>
> then it's OK.

I think so.  At least I find the new comment confusing, and I'd
prefer to see it cleaned up.

Thanks.