On Wed, Apr 14, 2021 at 08:12:34AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Apr 14 2021, Jeff King wrote:
>
> > On Tue, Apr 13, 2021 at 02:07:13PM -0700, Junio C Hamano wrote:
> >
> >> Jeff King <peff@xxxxxxxx> 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?
> >
> > I took "format specifier" to mean the "%" code within the format. E.g.:
> >
> > error("foo");
> >
> > has no format specifier, and thus no arguments after the format. But
> > every call will have at least the format string itself.
> >
> > AFAIK, portably using variadic macros means you need there to always be
> > at least one argument. Hence "error(fmt, ...)" is wrong (the "..." may
> > have no arguments) but "error(...)" is OK (you always have a format
> > string). I'm not sure if Ævar knows about some other portability gotcha,
> > or if he just didn't realize that this was written in the portable way.
>
> No, I just read elsewhere that GCC had non-standard behavior, and didn't
> look carefully at your implementation, but since it explicitly depended
> on GNUC etc. understood it to mean it was GCC-specific, not just
> C99-specific.
>
> So it can simply be changed to depend on HAVE_VARIADIC_MACROS instead?
I think it probably could be, yes.
The original predates HAVE_VARIADIC_MACROS (which we got in 2014); the
original error() macro is from e208f9cc75 (make error()'s constant
return value more visible, 2012-12-15).
The original also used the gcc-ism with the paste operator (see the
commit message for mention of it), but that was actually dropped later
by 9798f7e5f9 (Use __VA_ARGS__ for all of error's arguments,
2013-02-08), giving us the C99-portable version we have now.
All that said, I don't see much value in converting it to use
HAVE_VARIADIC_MACROS. It is mostly there to benefit gcc's warning code.
In theory making the return value visible could help other compilers
generate better code, too. If we care about doing that, we could switch
to making it unconditional. But aside from the variadic macros, it's
kind of a weird construct and I'd be worried about generating warnings
on other compilers or static analysis systems. E.g., see the hack from
87fe5df365 (inline constant return from error() function, 2014-05-06).
-Peff
