Closed Bug 1008360 Opened 5 years ago Closed 5 years ago

clang warning (treated as error) in disable-unified build: LayerSorter.cpp:156:18: warning: unused variable 'BLACK' [-Wunused-const-variable], and many more

Categories

(Core :: Graphics: Layers, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

STR:
 Build with ac_add_options --disable-unified-compilation and recent clang.
 (I'm using clang 3.5)

ACTUAL RESULTS:
{
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'BLACK'
 0:21.98 $SRCDIR/gfx/layers/LayerSorter.cpp:156:18: warning: unused variable 'BLACK' [-Wunused-const-variable]
 0:21.98 static const int BLACK = 0;
 0:21.98                  ^
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'RED'
 0:21.98 $SRCDIR/gfx/layers/LayerSorter.cpp:157:18: warning: unused variable 'RED' [-Wunused-const-variable]
 0:21.98 static const int RED = 1;
 0:21.98                  ^
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'GREEN'
 0:21.98 $SRCDIR/gfx/layers/LayerSorter.cpp:158:18: warning: unused variable 'GREEN' [-Wunused-const-variable]
 0:21.98 static const int GREEN = 2;
 0:21.98                  ^
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'YELLOW'
 0:21.98 $SRCDIR/gfx/layers/LayerSorter.cpp:159:18: warning: unused variable 'YELLOW' [-Wunused-const-variable]
 0:21.98 static const int YELLOW = 3;
 0:21.98                  ^
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'BLUE'
 0:21.98 $SRCDIR/gfx/layers/LayerSorter.cpp:160:18: warning: unused variable 'BLUE' [-Wunused-const-variable]
 0:21.98 static const int BLUE = 4;
 0:21.98                  ^
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'MAGENTA'
 0:21.98 $SRCDIR/gfx/layers/LayerSorter.cpp:161:18: warning: unused variable 'MAGENTA' [-Wunused-const-variable]
 0:21.98 static const int MAGENTA = 5;
 0:21.98                  ^
 0:21.98 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'CYAN'
 0:21.99 $SRCDIR/gfx/layers/LayerSorter.cpp:162:18: warning: unused variable 'CYAN' [-Wunused-const-variable]
 0:21.99 static const int CYAN = 6;
 0:21.99                  ^
 0:21.99 Warning: -Wunused-const-variable in $SRCDIR/gfx/layers/LayerSorter.cpp: unused variable 'WHITE'
 0:21.99 $SRCDIR/gfx/layers/LayerSorter.cpp:163:18: warning: unused variable 'WHITE' [-Wunused-const-variable]
 0:21.99 static const int WHITE = 7;
 0:21.99                  ^
}

I believe these warnings are treated as errors, if you have --enable-warnings-as-errors in your mozconfig.
Summary: clang warning (treated as error) in disable-unified build: LayerSorter.cpp:156:18: warning: unused variable 'BLACK' [-Wunused-const-variable] → clang warning (treated as error) in disable-unified build: LayerSorter.cpp:156:18: warning: unused variable 'BLACK' [-Wunused-const-variable], and many more
So there are several issues here:
 (1) these constants are only used inside of the #ifdef USE_XTERM_COLORING, so they should be defined down there
 (2) In builds with USE_XTERM_COLORING is defined, most of the constants are still unused and will still spam these build warnings (errors)

Patch coming up to address both of these.
Attached patch fix v1Splinter Review
This patch:
 (a) moves the variables into the #ifdef where they're used (or some of them are used, anyway)
 (b) comments out the variables that aren't used, so that they're there for documentation purposes, but the compiler won't complain about them.
 (c) adds constants XTERM_FOREGROUND_COLOR_OFFSET and XTERM_BACKGROUND_COLOR_OFFSET to clarify what the "30" and "40" magic numbers are there for.
Attachment #8420290 - Flags: review?(bugmail.mozilla)
I verified that this builds correctly when USE_XTERM_COLORING is explicitly defined & also when it's not-defined, BTW.
Comment on attachment 8420290 [details] [diff] [review]
fix v1

Review of attachment 8420290 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks.
Attachment #8420290 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/7e05c20bc99e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
See Also: → 1032644
You need to log in before you can comment on or make changes to this bug.