From: Karthik Nayak <karthik.188@gmail.com>
Date: Tue, 06 Jan 2026 16:28:15 GMT
Subject: Re: [PATCH v2 1/3] t/unit-tests: update clar to 39f11fe
Message-ID: <CAOLa=ZTd7Tq6jPCefua07Rw1-zyOY98tjR2g-Em6CM8wui4KtQ@mail.gmail.com>
In-Reply-To: <aVzvDGVEI2qVJv2F@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Tue, Jan 06, 2026 at 02:59:21AM -0800, Karthik Nayak wrote:
>> 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.
>

Agreed.

>> > @@ -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

Ah right. Thanks for the explanation.