On Fri, Jul 10, 2026 at 04:48:58PM -0400, Gregory Price wrote:
> On Mon, Jun 29, 2026 at 01:23:41PM +0100, Lorenzo Stoakes wrote:
> > -#define ASSERT_TRUE(_expr) \
> > - do { \
> > - if (!(_expr)) { \
> > - fprintf(stderr, \
> > - "Assert FAILED at %s:%d:%s(): %s is FALSE.\n", \
> > - __FILE__, __LINE__, __FUNCTION__, #_expr); \
> > - return false; \
> > - } \
> > +#define __ASSERT_TRUE(_expr, _fmt, ...)
> > \
> > + do { \
> > + if (!(_expr)) { \
> > + fprintf(stderr, \
> > + "Assert FAILED at %s:%d:%s(): %s is FALSE" \
> > + _fmt ".\n", \
> > + __FILE__, __LINE__, __FUNCTION__, #_expr \
> > + __VA_OPT__(,) __VA_ARGS__); \
> > + return false; \
> > + } \
> > } while (0)
> >
> > +#define __TO_SCALAR(x) ((unsigned long long)(uintptr_t)(x))
> > +
> > +#define ASSERT_TRUE(_expr) __ASSERT_TRUE(_expr, "")
>
> Mmmmm... macro madness.... I don't think this is what you want.
>
> I think you end up double-running the expression in the failure branch.
>
> ASSERT_EQ(cleanup_mm(&mm, &vmi), 2)
>
> run through the preprocessor expands to:
>
> do {
> if (!( (cleanup_mm(&mm, &vmi)) == (2) )) {
> **** first run ****
>
> fprintf(stderr,
> "Assert FAILED at %s:%d:%s(): %s is FALSE" " (0x%llx !=
> 0x%llx)" ".\n",
> "merge.c", 645, __FUNCTION__,
> "(cleanup_mm(&mm, &vmi)) == (2)",
> ((unsigned long long)(uintptr_t)(cleanup_mm(&mm, &vmi))),
> **** second run ****
>
> ((unsigned long long)(uintptr_t)(2)));
> return false;
> }
> } while (0);
>
>
> A bunch of existing ASSERT callers mutate state, so there's no guarantee
> the printed value matches teh actual test value.
>
> I think you want something like:
>
> #define ASSERT_EQ(_val1, _val2) do { \
> __auto_type _v1 = (_val1); \
> __auto_type _v2 = (_val2); \
> __ASSERT_TRUE(_v1 == _v2, " (0x%llx != 0x%llx)", \
> __TO_SCALAR(_v1), __TO_SCALAR(_v2)); \
> } while (0)
>
> which expands to:
>
> do {
> __auto_type _v1 = (cleanup_mm(&mm, &vmi));
> __auto_type _v2 = (2);
> do {
> if (!(_v1 == _v2)) {
> fprintf(stderr, "...FALSE (0x%llx != 0x%llx).\n",
> "merge.c", 645, __FUNCTION__, "_v1 == _v2",
> ((unsigned long long)(uintptr_t)(_v1)),
> ((unsigned long long)(uintptr_t)(_v2)));
> return false;
> }
> } while (0);
> } while (0);
>
> ~Gregory
It's funny you should mention that... fixed in v2.
But ugh sorry that you reviewed this while I was also fixing this up (Claude
reported it also), your review's very appreciated :>)
Cheers, Lorenzo
