Closed
Bug 577905
Opened 14 years ago
Closed 14 years ago
mark DEBUG only variables as ifdef DEBUG in svg
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla2.0b8
People
(Reporter: timeless, Assigned: timeless)
References
Details
Attachments
(1 file, 2 obsolete files)
2.67 KB,
patch
|
timeless
:
review+
roc
:
approval2.0+
|
Details | Diff | Splinter Review |
this is part of a crusade to get rid of compilation warnings
Attachment #456731 -
Attachment is obsolete: true
Attachment #456732 -
Flags: review?(dholbert)
Attachment #456731 -
Flags: review?(dholbert)
Comment 3•14 years ago
|
||
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+
Updated•14 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 4•14 years ago
|
||
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•14 years ago
|
||
(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.
Attachment #456732 -
Attachment is obsolete: true
Attachment #463490 -
Flags: review+
err no one *stood* up for me.
Summary: use NS_DEBUG_ASSIGN for svg → mark DEBUG only variables as ifdef DEBUG in svg
Comment 8•14 years ago
|
||
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•14 years ago
|
Attachment #463490 -
Attachment description: using #ifdef DEBUG → using #ifdef DEBUG (r=dholbert)
Attachment #463490 -
Flags: approval2.0+
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•14 years ago
|
||
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.
Keywords: checkin-needed
![]() |
||
Comment 11•14 years ago
|
||
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
Comment 12•14 years ago
|
||
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•14 years ago
|
||
Closing given comment 12, then :)
Status: ASSIGNED → RESOLVED
Closed: 14 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.
Description
•