Add FAIL_ON_WARNINGS in layout/tables

RESOLVED FIXED in mozilla12



Layout: Tables
6 years ago
5 years ago


(Reporter: dholbert, Assigned: dholbert)



Firefox Tracking Flags

(Not tracked)



(1 attachment)

Now that bug 703121 has landed, we can start labeling gcc-warning-free directories as FAIL_ON_WARNINGS, to keep them warning-free.

Filing this bug on adding that label in layout/tables, which is already gcc-warning-free (both opt & debug) on my system. (discounting -Wuninitialized warnings which we're ignoring for the purposes of warnings-as-errors)
Created attachment 587199 [details] [diff] [review]
patch v1

I've confirmed that this errors out in --enable-warnings-as-errors builds, if I introduce code in layout/tables that causes build warnings. (i.e. this patch does what it's intended to do)

Currently running this through Try (along with bug 716699) as a sanity check:
Attachment #587199 - Flags: review?(dbaron)
Comment on attachment 587199 [details] [diff] [review]
patch v1

Canceling review for now -- it looks like the "-Wno-error=uninitialized" from bug 716541 is ineffective on mac, probably because we use an old version of GCC there (g++-4.2), so we do hit a "may be used uninitialized" warning and treat it as an error on Mac opt:

Waiting on this bug's patch until that's fixed.
Attachment #587199 - Flags: review?(dbaron)
Actually, I might have mis-identified the source of that build failure.

The text "cc1plus: warnings being treated as errors" does appear before some uninitialized-variable warnings, but then there are a bunch of lines like...
> {standard input}:16725:symbol: "L__ZN9nsGkAtoms10tableFrameE$non_lazy_ptr" can't be undefined in a subtraction expression
> {standard input}:unknown:Undefined local symbol L__ZN9nsGkAtoms16bcTableCellFrameE$non_lazy_ptr
...before the build actually errors out.  So maybe those were the source of the build failure. (I'll investigate some more.)
Depends on: 703121
Not sure about that earlier build error yet.

One other update though -- as a sanity-check, I verified (in a separate try push) that introducing an unused variable in /layout/tables will indeed trigger a build error, on Try:
> BasicTableLayoutStrategy.cpp:80:9: error: unused variable 'unused'
(that's a variable I added as a test)
(I'd already verified that locally, per comment 1, but good to see that it works on Try, too. :))
I filed 716787 on the mac issue brought up in comment 2 - comment 3.
Comment on attachment 587199 [details] [diff] [review]
patch v1

This can't land until bug 716787 is sorted out, but in the meantime, might as well request review.  (It's a one-liner.)

Try push in comment 1 shows that it builds successfully, aside from the OS X redness due to bug 716787.  Try push in comment 4 shows that it successfully catches newly-introduced build warnings.
Attachment #587199 - Flags: review?(dbaron)
(bug 716787 is now fixed, so this is ready to land as soon as it gets r+.)
Hardware: x86_64 → All
Comment on attachment 587199 [details] [diff] [review]
patch v1

Attachment #587199 - Flags: review?(dbaron) → review+
Target Milestone: --- → mozilla12
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.