577905 - mark DEBUG only variables as ifdef DEBUG in svg

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[timeless]

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

Comment 1

[timeless]

Attachment: patch

Comment 2

[timeless]

Attachment: patch

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 456732 [details] [diff] [review]
patch

># HG changeset patch
># Parent c51a1d79202b80c093ab86fca2617104f6484e1f
># User [e-mail]
>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)");

Comment 4

[Daniel Holbert [:dholbert]]

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)

Comment 5

[Daniel Holbert [:dholbert]]

(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)

Comment 6

[timeless]

Attachment: using #ifdef DEBUG (r=dholbert)

sorry, no one stupid up for me.

Comment 7

[timeless]

err no one *stood* up for me.

Comment 8

[Daniel Holbert [:dholbert]]

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.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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");

Comment 10

[Daniel Holbert [:dholbert]]

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.

Comment 11

[Jonathan Watt [:jwatt]]

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.

Comment 12

[Daniel Holbert [:dholbert]]

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.

Comment 13

[Phil Ringnalda (:philor)]

Closing given comment 12, then :)