968635 - build bustage using clang & --enable-warnings-as-errors: "TextureClientX11.h:59:16: error: private field 'mTextureFlags' is not used [-Werror,-Wunused-private-field]"

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Daniel Holbert [:dholbert]]

Bug 947045 just landed on inbound, adding ./gfx/layers/basic/TextureClientX11.h, and seems to have caused this build bustage on my local machine with clang++ and ac_add_options --enable-warnings-as-errors:
{
gfx/layers/basic/TextureClientX11.cpp:5:
../../dist/include/mozilla/layers/TextureClientX11.h:59:16: error: private field 'mTextureFlags' is not used [-Werror,-Wunused-private-field]
}

Maybe we can just drop that variable?

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> Maybe we can just drop that variable?

Looking at the patch, attachment 8370505 [details] [diff] [review], it looks like we can just drop it (and the constructor-param "aFlags" along with it).

Bill, would that make sense?

Comment 2

[Daniel Holbert [:dholbert]]

(or alternately, maybe we really should be checking this variable but aren't?)

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

It seems like we ought to be able to take it out, but I don't know this code very well. Let's ask Matt.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: strawman (and/or local hackaround): drop the variable

Here's a strawman patch to drop the variable, per comment 1.

(Even if this isn't what we want, this can serve as a local hackaround for anyone else who's running into this, in the meantime.)

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 8371243 [details] [diff] [review]
strawman (and/or local hackaround): drop the variable

(er, never mind about this exact patch; my build failed when it got to CompositableClient.cpp, which invokes the constructor with an explicit flags arg)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: strawman/hackaround

(here's a working hackaround, not modifying the constructor this time)

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: texture-flags-fix

This was actually a real bug. We need to set the flags in the base class.

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b673cf81b9e5

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b673cf81b9e5