mark DEBUG only variables as ifdef DEBUG in svg

RESOLVED FIXED in mozilla2.0b8

Status

()

Core
SVG
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
mozilla2.0b8
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
this is part of a crusade to get rid of compilation warnings
(Assignee)

Comment 1

8 years ago
Created attachment 456731 [details] [diff] [review]
patch
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #456731 - Flags: review?(dholbert)
(Assignee)

Comment 2

8 years ago
Created attachment 456732 [details] [diff] [review]
patch
Attachment #456731 - Attachment is obsolete: true
Attachment #456732 - Flags: review?(dholbert)
Attachment #456731 - Flags: review?(dholbert)
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)");
Attachment #456732 - Flags: review?(dholbert) → review+
OS: Mac OS X → All
Hardware: x86 → All
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)
(Assignee)

Comment 6

8 years ago
Created attachment 463490 [details] [diff] [review]
using #ifdef DEBUG (r=dholbert)

sorry, no one stupid up for me.
Attachment #456732 - Attachment is obsolete: true
Attachment #463490 - Flags: review+
(Assignee)

Comment 7

8 years ago
err no one *stood* up for me.
Summary: use NS_DEBUG_ASSIGN for svg → mark DEBUG only variables as ifdef DEBUG in svg
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.

Updated

8 years ago
Attachment #463490 - Attachment description: using #ifdef DEBUG → using #ifdef DEBUG (r=dholbert)
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.
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
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.
Keywords: checkin-needed
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 :)
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.