Re: [PATCH v2 1/3] t/unit-tests: update clar to 39f11fe

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 6, 2026, 11:16 UTC
Message-ID: <aVzvDGVEI2qVJv2F@pks.im>
In-Reply-To: <CAOLa=ZQZnYVuK8mDi6Yb8_+hqw_TMugn6i7BJCj1gbNHOruNWA@mail.gmail.com>

On Tue, Jan 06, 2026 at 02:59:21AM -0800, Karthik Nayak wrote:

Show 9 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Update clar to commit 39f11fe (Merge pull request #131 from
> > pks-gitlab/pks-integer-double-evaluation, 2025-12-05). This commit
> > includes the following changes relevant to Git:
> >
> 
> Nit: There is a newer commit merged into the clar repository, but I
> don't think it is so important to include.

Yeah, I don't really think it's necessary. If this series needs a reroll I'll include it, but otherwise I'll keep this series as-is.

Show 10 quoted lines

> > @@ -149,6 +150,7 @@ const char *cl_fixture_basename(const char *fixture_name);
> >   * Forced failure/warning
> >   */
> >  #define cl_fail(desc) clar__fail(CLAR_CURRENT_FILE, CLAR_CURRENT_FUNC, CLAR_CURRENT_LINE, "Test failed.", desc, 1)
> > +#define cl_failf(desc,...) clar__failf(CLAR_CURRENT_FILE, CLAR_CURRENT_FUNC, CLAR_CURRENT_LINE, 1, "Test failed.", desc, __VA_ARGS__)
> 
> Nit: While most of the function accept description with variable
> arguments, this is the only one which has the '...f()' format explicitly
> separated out. It would be nicer if we simply make this part of
> 'cl_fail()', no?

The problem is that we cannot do so easily. Varargs require at least one argument to be present, so we cannot make this `cl_fail(desc, ...)` without breaking the case where there are no variable arguments:

  In file included from ../t/unit-tests/clar/clar.c:1053:
  ../t/unit-tests/clar/clar/fs.h:460:3: error: expected expression
    460 |                 cl_fail("Cannot copy; cannot stat destination");
        |                 ^
  ../t/unit-tests/clar/clar.h:152:132: note: expanded from macro 'cl_fail'
    152 | #define cl_fail(desc,...) clar__failf(CLAR_CURRENT_FILE, CLAR_CURRENT_FUNC, CLAR_CURRENT_LINE, 1, "Test failed.", desc, __VA_ARGS__)
        |                                                                                                                                    ^

The alternative would be to make this `cl_fail(...)` instead, but to the best of my knowledge this isn't even a valid construct.

Patrick