1588015 - Provide alternative PR_ASSERT that "uses" inputs always

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement, P2)

Description

[Martin Thomson [:mt:]]

Technically, this isn't a problem, but it creates some odd cases. Say that I wanted to assert the return value of a function, like this bit in certdb.c:

#ifdef DEBUG
    {
        PRStatus prstat = PZ_Unlock(certTempPermLock);
        PORT_Assert(prstat == PR_SUCCESS); // PORT_Assert is defined as PR_ASSERT
    }
#else
    (void)PZ_Unlock(certTempPermLock);
#endif

This is pretty ugly, but you get unused variable warnings if you simplify to:

    PRStatus prstat = PZ_Unlock(certTempPermLock);
    PORT_Assert(prstat == PR_SUCCESS);

You could change the definition of PR_ASSERT to this:

#define PR_ASSERT(expr) ((void)(0 && (expr)))

Which optimizes out the expression easily (-01 is enough in gdb), but we have code that does this:

#ifdef DEBUG
    int x = 0;
#endif
   // ... 
#ifdef DEBUG
    x = something;
#endif
   // ...
    PR_ASSERT(x);

Which fails even harder because x is undefined on the unguarded PR_ASSERT invocation. That code should probably just guard the assertion call as well, but there's plenty of code built this way already and we can't break that code.

So I'm suggesting that we add a macro that "uses" its argument. I'm suggesting it here rather than just for NSS, because that has broader applicability, but I will also open an NSS to move to this model or to use this model of assertion (PORT_Assert is also subject to the same constraints as PR_ASSERT, so we have the same problem there).

Comment 1

[Marcus Burghardt]

Attachment: Bug 1588015 - Provide alternative PR_ASSERT that uses inputs always. r=mt

Comment 2

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/projects/nspr/rev/b8aa9ae7989fe73b8e152b71f2d2cc78d31067c9