576052 - Rename NS_ABORT_IF_FALSE() to NS_ASSERT()

Status: RESOLVED WORKSFORME

(Core :: XPCOM, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Reason 1: the "IF_FALSE" part just adds cognitive overhead.  I get the condition inverted for ABORT_IF_FALSE() way more than I ever did with assert().

Reason 2: coming from an "assert()" background, I found it nonobvious that ABORT_IF_FALSE() was really what I wanted, and not NS_ASSERTION().

Bobby argues (from the perspective of only wanting fatal asserts from here on) that having ASSERT() and ASSERTION() coexist will lead to confusion and coders/reviewers possibly choosing ASSERTION() when they really wanted ASSERT().  That's a fair point, but my (glib) counter is that s/NS_ASSERTION/NS_ASSERT/ should be next on the list (once tinderbox assertions are fixed).  In the meantime, I think people are more likely to use NS_ASSERTION when they really want NS_ABORT_IF_FALSE, so I see the glass as half-full --- I think confusion around ASSERT and ASSERTION will lead to more fatal asserts.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

I think the potential confusion is a worse problem than the current cognitive overhead. If "fixing assertions" is going to happen as soon as you seem to think it will (I'm not convinced!), then living with the cognitive overhead for just a bit longer won't be so bad, will it? :)

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The cool kids use MOZ_ASSERT() nowadays.