From: Patrick Steinhardt Date: Tue, 06 Jan 2026 11:16:28 GMT Subject: Re: [PATCH v2 1/3] t/unit-tests: update clar to 39f11fe Message-ID: In-Reply-To: On Tue, Jan 06, 2026 at 02:59:21AM -0800, Karthik Nayak wrote: > Patrick Steinhardt 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. > > @@ -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