this is part of a crusade to get rid of compilation warnings

Comment on attachment 456732 [details] [diff] [review] patch ># HG changeset patch ># Parent c51a1d79202b80c093ab86fca2617104f6484e1f ># User timeless@mozdev.org >Bug 577905 use NS_DEBUG_ASSIGN for svg >r=dholbert > >diff --git a/content/smil/nsSMILAnimationFunction.cpp b/content/smil/nsSMILAnimationFunction.cpp >--- a/content/smil/nsSMILAnimationFunction.cpp >+++ b/content/smil/nsSMILAnimationFunction.cpp >@@ -546,7 +546,7 @@ nsSMILAnimationFunction::ComputePacedPos > NS_ASSERTION(remainingDist >= 0, "distance values must be non-negative"); > > double curIntervalDist; >- nsresult rv = aValues[i].ComputeDistance(aValues[i+1], curIntervalDist); >+ NS_DEBUG_ASSIGN(nsresult rv) aValues[i].ComputeDistance(aValues[i+1], curIntervalDist); > NS_ABORT_IF_FALSE(NS_SUCCEEDED(rv), > "If we got through ComputePacedTotalDistance, we should " > "be able to recompute each sub-distance without errors"); >diff --git a/content/svg/content/src/nsSVGAttrTearoffTable.h b/content/svg/content/src/nsSVGAttrTearoffTable.h >--- a/content/svg/content/src/nsSVGAttrTearoffTable.h >+++ b/content/svg/content/src/nsSVGAttrTearoffTable.h >@@ -75,7 +75,7 @@ nsSVGAttrTearoffTable<SimpleType, Tearof > return nsnull; > > TearoffType *tearoff = nsnull; >- PRBool found = mTable.Get(aSimple, &tearoff); >+ NS_DEBUG_ASSIGN(PRBool found) mTable.Get(aSimple, &tearoff); > NS_ABORT_IF_FALSE(!found || tearoff, > "NULL pointer stored in attribute tear-off map"); > >@@ -98,7 +98,7 @@ nsSVGAttrTearoffTable<SimpleType, Tearof > return; > } > >- PRBool result = mTable.Put(aSimple, aTearoff); >+ NS_DEBUG_ASSIGN(PRBool result) mTable.Put(aSimple, aTearoff); > NS_ABORT_IF_FALSE(result, "Out of memory."); > } > >diff --git a/content/svg/content/src/nsSVGElement.cpp b/content/svg/content/src/nsSVGElement.cpp >--- a/content/svg/content/src/nsSVGElement.cpp >+++ b/content/svg/content/src/nsSVGElement.cpp >@@ -1259,9 +1259,11 @@ nsSVGElement::UpdateAnimatedContentStyle > animContentStyleRule(mappedAttrParser.CreateStyleRule()); > > if (animContentStyleRule) { >- nsresult rv = SetProperty(SMIL_MAPPED_ATTR_ANIMVAL, >- SMIL_MAPPED_ATTR_STYLERULE_ATOM, >- animContentStyleRule.get(), ReleaseStyleRule); >+ NS_DEBUG_ASSIGN(nsresult rv) >+ SetProperty(SMIL_MAPPED_ATTR_ANIMVAL, >+ SMIL_MAPPED_ATTR_STYLERULE_ATOM, >+ animContentStyleRule.get(), >+ ReleaseStyleRule); > animContentStyleRule.forget(); > NS_ABORT_IF_FALSE(rv == NS_OK, > "SetProperty failed (or overwrote something)");

WTH... sorry for comment 3 -- the new bugzilla version apparently thought I did "Edit attachment as comment", but I swear I didn't. Here's what I actually typed in the comment field and thought I was posting as Comment 3: ---- timeless++!! This is much cleaner than then effectively having the expanded macro everywhere, which is what we do in a lot of places right now. At some point it'd probably be worth doing a MXR search for "#ifdef DEBUG" to catch those other places, as potential clients of this new macro. But that's more work and could easily be a separate patch and/or bug. r=dholbert (assuming bug 577899 passes review)

(In reply to comment #4) > the new bugzilla version apparently thought I did > "Edit attachment as comment", but I swear I didn't. (Ah, that's bug 577814, FWIW)

sorry, no one stupid up for me.

err no one *stood* up for me.

Comment on attachment 463490 [details] [diff] [review] using #ifdef DEBUG (r=dholbert) #ifdef DEBUG version looks good to me. Sorry that bug 577899 didn't work out.

Actually, you know what would be great? Some macro like NS_DEBUG_DECL(decl) that expands to "decl" in DEBUG builds and nothing in other builds. Then you could write NS_DEBUG_DECL(nsresult rv =) Foo(); NS_ASSERTION(NS_SUCCEEDED(rv), "Blah");

That was sort of what timeless had in bug 577814 (and the first patch here), but it kind of got vetoed in bug 577814 comment 10 & bug 577814 comment 17.

Pushed http://hg.mozilla.org/mozilla-central/rev/f6dbfac1ab22 Leaving this open given comment 9, but maybe that should be given a fresh attempt in a fresh bug.

Sorry, I meant(In reply to comment #10) > That was sort of what timeless had in bug 577814 (and the first patch here), > but it kind of got vetoed in bug 577814 comment 10 & bug 577814 comment 17. Sorry, copy-paste fail. I meant bug 577899, bug 577899 comment 10, bug 577899 comment 17.

Closing given comment 12, then :)