Closed
Bug 1299727
Opened 7 years ago
Closed 7 years ago
Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
178.05 KB,
patch
|
erahm
:
review+
|
Details | Diff | Splinter Review |
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.
![]() |
Assignee | |
Comment 1•7 years ago
|
||
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.
Attachment #8787054 -
Flags: review?(erahm)
Comment 2•7 years ago
|
||
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.
Attachment #8787054 -
Flags: review?(erahm) → review+
![]() |
Assignee | |
Comment 3•7 years ago
|
||
> > + 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.
![]() |
Assignee | |
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/93657f8dedaafa384bcd49c71b52e195ec968de8 Bug 1299727 - Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION. r=erahm.
Pushed by nnethercote@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/93657f8dedaa Rename NS_WARN_IF_FALSE as NS_WARNING_ASSERTION. r=erahm.
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/93657f8dedaa
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•