The default bug view has changed. See this FAQ.

Add FAIL_ON_WARNINGS in layout/tables

RESOLVED FIXED in mozilla12

Status

()

Core
Layout: Tables
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla12
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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)
(Assignee)

Comment 1

5 years ago
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:
 https://tbpl.mozilla.org/?tree=Try&rev=5fb8ea0712d7
Attachment #587199 - Flags: review?(dbaron)
(Assignee)

Comment 2

5 years ago
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:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dholbert@mozilla.com-5fb8ea0712d7/try-macosx-debug/try-macosx-debug-build6451.txt.gz

Waiting on this bug's patch until that's fixed.
Attachment #587199 - Flags: review?(dbaron)
(Assignee)

Comment 3

5 years ago
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
...and...
> {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.)
(Assignee)

Updated

5 years ago
Depends on: 703121
(Assignee)

Comment 4

5 years ago
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)
 https://tbpl.mozilla.org/php/getParsedLog.php?id=8432358&tree=Try
(Assignee)

Comment 5

5 years ago
(I'd already verified that locally, per comment 1, but good to see that it works on Try, too. :))
(Assignee)

Comment 6

5 years ago
I filed 716787 on the mac issue brought up in comment 2 - comment 3.
(Assignee)

Comment 7

5 years ago
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)
(Assignee)

Comment 8

5 years ago
(bug 716787 is now fixed, so this is ready to land as soon as it gets r+.)
(Assignee)

Updated

5 years ago
Hardware: x86_64 → All
Comment on attachment 587199 [details] [diff] [review]
patch v1

r=dbaron
Attachment #587199 - Flags: review?(dbaron) → review+
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d0f4e08e31e
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/5d0f4e08e31e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.