1299727 - Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nicholas Nethercote [inactive]]

The new name makes the sense of the condition much clearer. E.g. compare:

  NS_WARN_IF_FALSE(!rv.Failed());

with:

  NS_WARNING_ASSERTION(!rv.Failed());

The new name also makes it clearer that it only has effect in debug builds,
because that's standard for assertions.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION

I'd love to use NS_WARN_ASSERTION or NS_WARNING_ASSERT or even NS_WARN_ASSERT
to reduce the number of line breaks required... but NS_WARNING_ASSERTION really
feels like the best name for this.

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8787054 [details] [diff] [review]
Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION

Review of attachment 8787054 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

We're definitely going to need to send out a message to dev-platform for this as well as update any MDN docs.

::: dom/ipc/Blob.cpp
@@ +1604,5 @@
>      nsCOMPtr<nsIThread> ioTarget;
>      mIOTarget.swap(ioTarget);
>  
> +    NS_WARNING_ASSERTION(NS_SUCCEEDED(stream->Close()),
> +                         "Failed to close stream!");

Follow up: stream->Close() in debug only assert.

Comment 3

[Nicholas Nethercote [inactive]]

> > +    NS_WARNING_ASSERTION(NS_SUCCEEDED(stream->Close()),
> > +                         "Failed to close stream!");
> 
> Follow up: stream->Close() in debug only assert.

Oh snap! Nice catch. That adds weight to the "clearer that it only has effect in debug builds" argument from comment 0. In fact, bug 994190 did exactly this:

> -    nsresult rv = stream->Close();
> -    NS_ENSURE_SUCCESS(rv, rv);
> +    NS_WARN_IF_FALSE(NS_SUCCEEDED(stream->Close()), "Failed to close stream!");

<sadface>

I filed bug 1300007 to fix it.

Comment 4

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/93657f8dedaafa384bcd49c71b52e195ec968de8
Bug 1299727 - Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION. r=erahm.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/93657f8dedaa
Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION. r=erahm.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/93657f8dedaa