525143 - Add a hard-abort NS_ASSERT(cond, fmt, ...) that trips the Windows debugger

Status: RESOLVED WONTFIX

(Core :: XPCOM, enhancement)

Description

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

I prefer NS_ABORT_IF_FALSE() to NS_ASSERTION(), but "ABORT_IF_FALSE" is a more verbose and confusing name.  I get the asserted condition backwards much more frequently than I do with |assert()|.  Old dog etc.

In behavior, NS_ASSERT() would be like |assert()|, or IOW approximately an amalgamation of NS_ABORT_IF_FALSE() and NS_ASSERTION().  I understand that NS_ABORT_IF_FALSE() doesn't trip the Windows debugger; NS_ASSERT() would, a la Windows CRT |assert()|.

Also, it would be great if NS_ASSERT() took a format string and variadic argument list, a la printf(), rather than a message param.  I understand that JS uses variadic macros, so we already require compilers to support that.

Any objections?

Comment 1

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

Before this gets DUP'd, I should point out that what I have in mind is different that bug 96699.

Comment 2

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

Attachment: v1

The use case for NS_ASSERT() is every place I've written NS_ABORT_IF_FALSE() for the last several months.  And where I've wanted printf-style debugging output with failed assertions.

Comment 3

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

Attachment: v1 \ typos

Oops, while waiting for another build to finish I saw that this first patch was slightly off.

Comment 4

[Benjamin Smedberg]

Comment on attachment 412057 [details] [diff] [review]
v1 \ typos

We only require variadic macros when the JIT is enabled, so we don't use them on various compilers that we still support. I'd prefer, if you want richer formatting, to go with Chromium-style log objects and << operators, which I'm currently quite enamored of. That was you can write NS_ASSERT(condition); // or
NS_ASSERT(condition) << "more information: " << i;

I don't really understand the difference between NS_DEBUG_ABORT and NS_DEBUG_ASSERT. Why can't we just make _ABORT break into a debugger in DEBUG builds and abort in non-debug builds (so that end-users get a crash reporter, but developers get MSVC)? Of course, I don't really understand NS_DEBUG_BREAK either, since that seems to be roughly the same as NS_DEBUG_ASSERTION.

Comment 5

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

(In reply to comment #4)
> (From update of attachment 412057 [details] [diff] [review])
> We only require variadic macros when the JIT is enabled, so we don't use them
> on various compilers that we still support. I'd prefer, if you want richer
> formatting, to go with Chromium-style log objects and << operators, which I'm
> currently quite enamored of. That was you can write NS_ASSERT(condition); // or
> NS_ASSERT(condition) << "more information: " << i;
> 

I fine with this.  The whole point of NS_ASSERT() is to have an assertion macro with libc |assert()| semantics.  Coming to Mozilla after having written O(10^5) lines of C, I found NS_ASSERTION/NS_ABORT_IF_FALSE rather confusing, for no net gain.

> I don't really understand the difference between NS_DEBUG_ABORT and
> NS_DEBUG_ASSERT. Why can't we just make _ABORT break into a debugger in DEBUG
> builds and abort in non-debug builds (so that end-users get a crash reporter,
> but developers get MSVC)? Of course, I don't really understand NS_DEBUG_BREAK
> either, since that seems to be roughly the same as NS_DEBUG_ASSERTION.

I don't have *any* interest in messing with these guys, because of backwards compatibility issues.  Feel free! ;)