Last Comment Bug 716699 - Add FAIL_ON_WARNINGS in some SVG/SMIL directories
: Add FAIL_ON_WARNINGS in some SVG/SMIL directories
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Daniel Holbert [:dholbert]
:
:
Mentors:
Depends on: 703121
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-09 14:50 PST by Daniel Holbert [:dholbert]
Modified: 2012-01-12 10:56 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (1.65 KB, patch)
2012-01-09 16:13 PST, Daniel Holbert [:dholbert]
jwatt: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-01-09 14:50:40 PST
Now that bug 703121 has landed, we can start labeling gcc-warning-free directories as FAIL_ON_WARNINGS, to keep them warning-free.

I'm filing this bug on adding this label for already-warning-free SVG-related directories, which I believe to be the following:
 content/smil
 content/svg/document/src/
 layout/svg/base/src/

(I'm not including content/svg/content because I seem to recall it having a few GCC warnings, but I'll double-check that and include it if it's warning-free.)
Comment 1 Daniel Holbert [:dholbert] 2012-01-09 15:45:20 PST
(In reply to Daniel Holbert [:dholbert] from comment #0)
> (I'm not including content/svg/content because I seem to recall it having a
> few GCC warnings, but I'll double-check that and include it if it's
> warning-free.)

Confirmed -- I tried building with warnings-as-errors on in content/svg/content (using g++ 4.6.2), and hit this:
> nsSVGFilters.cpp:3794:21: error: variable ‘yExt’ set but not used [-Werror=unused-but-set-variable]
(there could easily be more warnings as well; this is just the first one I hit)

So, I won't concern myself with content/svg for the purposes of this bug. (at least not yet)
Comment 2 Daniel Holbert [:dholbert] 2012-01-09 16:13:06 PST
Created attachment 587191 [details] [diff] [review]
patch v1

This patch covers the 3 directories mentioned in comment 1.  I've confirmed locally that it works (no errors) in a debug build, and I'm most of the way through a successful opt build.

Just pushed to TryServer for mac/linux/android (our warnings-as-errors-enabled platforms), too:
 https://tbpl.mozilla.org/?tree=Try&rev=5fb8ea0712d7
Comment 3 Daniel Holbert [:dholbert] 2012-01-10 12:37:56 PST
Comment on attachment 587191 [details] [diff] [review]
patch v1

This can't land until we've sorted out a mysterious Mac OS X build failure in bug 716787.  However, aside from that, it's ready for review.
Comment 4 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-01-10 15:10:46 PST
Comment on attachment 587191 [details] [diff] [review]
patch v1

Sweet!
Comment 5 Daniel Holbert [:dholbert] 2012-01-11 00:34:58 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/b121ed2a5db6
Comment 6 Ed Morley [:emorley] 2012-01-11 02:21:03 PST
Backed out of inbound for "[mangled variable name]$non_lazy_ptr can't be undefined in a subtraction expression" a la bug 716787.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4202e19f36e2
Comment 7 Daniel Holbert [:dholbert] 2012-01-11 12:48:59 PST
Looks like it just needed a clobber for bug 716787 to kick in and allow this to be non-red.
Re-landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/1d1938bb244d
Comment 8 Matt Brubeck (:mbrubeck) 2012-01-12 10:56:35 PST
https://hg.mozilla.org/mozilla-central/rev/1d1938bb244d

Note You need to log in before you can comment on or make changes to this bug.