900284 - Build failure with Clang 3.3 + --enable-warnings-as-errors: private fields 'mOurA11yNotification', 'mWasFrameVisible' are not used

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Alex Xu]

41:44.15 In file included from /home/alex/mozilla-central/layout/base/RestyleManager.cpp:11:
41:44.15 /home/alex/mozilla-central/layout/base/RestyleManager.h:387:24: error: private field 'mOurA11yNotification' is not used [-Werror,-Wunused-private-field]
41:44.15   A11yNotificationType mOurA11yNotification;
41:44.15                        ^
41:44.15 /home/alex/mozilla-central/layout/base/RestyleManager.h:389:8: error: private field 'mWasFrameVisible' is not used [-Werror,-Wunused-private-field]
41:44.15   bool mWasFrameVisible;
41:44.15        ^
41:44.17 nsHTMLButtonControlFrame.cpp
41:44.21 nsCSSKeywords.cpp
41:44.28 2 errors generated.
41:44.29 
41:44.29 In the directory  /home/alex/mozilla-central/obj-x86_64-unknown-linux-gnu/layout/base
41:44.29 The following command failed to execute properly:
41:44.29 /usr/bin/ccache clang++ -o RestyleManager.o -c -I../../dist/stl_wrappers -I../../dist/system_wrappers -include /home/alex/mozilla-central/config/gcc_hidden.h -DMOZ_GLUE_IN_PROGRAM -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT -DOS_POSIX=1 -DOS_LINUX=1 -D_IMPL_NS_LAYOUT -DHB_DONT_DEFINE_STDINT -I/home/alex/mozilla-central/ipc/chromium/src -I/home/alex/mozilla-central/ipc/glue -I../../ipc/ipdl/_ipdlheaders -I/home/alex/mozilla-central/layout/base -I/home/alex/mozilla-central/layout/base/../style -I/home/alex/mozilla-central/layout/base/../generic -I/home/alex/mozilla-central/layout/base/../forms -I/home/alex/mozilla-central/layout/base/../tables -I/home/alex/mozilla-central/layout/base/../printing -I/home/alex/mozilla-central/layout/base/../xul/base/src -I/home/alex/mozilla-central/layout/base/../xul/tree/ -I/home/alex/mozilla-central/layout/base/../../content/base/src -I/home/alex/mozilla-central/layout/base/../../content/events/src -I/home/alex/mozilla-central/layout/base/../../content/xbl/src -I/home/alex/mozilla-central/layout/base/../../view/src -I/home/alex/mozilla-central/layout/base/../../dom/base -I/home/alex/mozilla-central/layout/base/../../content/html/content/src -I/home/alex/mozilla-central/layout/base/../../content/svg/content/src -I/home/alex/mozilla-central/xpcom/ds -I/home/alex/mozilla-central/layout/base/../svg -I/home/alex/mozilla-central/layout/base/../mathml -I/home/alex/mozilla-central/layout/base -I. -I../../dist/include -I/home/alex/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/include/nspr -I/home/alex/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/include/nss -fPIC -Qunused-arguments -Qunused-arguments -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -march=native -O2 -pipe -march=native -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -I/home/alex/mozilla-central/build/unix/headers -pthread -pipe -DNDEBUG -DTRIMMED -g -Xclang -load -Xclang ../../build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fomit-frame-pointer -Werror -Wno-error=uninitialized -Wno-error=deprecated-declarations -I/home/alex/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/include/cairo -Qunused-arguments -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MP -MF .deps/RestyleManager.o.pp /home/alex/mozilla-central/layout/base/RestyleManager.cpp
41:44.31 gmake[6]: *** [RestyleManager.o] Error 1
41:44.31 gmake[5]: *** [base_libs] Error 2
41:44.31 gmake[5]: *** Waiting for unfinished jobs....


My guess is they're actually not used, but someone should probably check that out.

Building from fd03bb4d1e48 using Clang 3.3 on Gentoo Linux.

Comment 1

[Mats Palmgren (inactive)]

mOurA11yNotification is used, but only in #ifdef ACCESSIBILITY blocks,
except for the ctor initializers:
http://mxr.mozilla.org/mozilla-central/search?string=mOurA11yNotification
so wrapping those (and the member) in #ifdef ACCESSIBILITY should fix it.

same for mWasFrameVisible:
http://mxr.mozilla.org/mozilla-central/search?string=mWasFrameVisible

Comment 2

[Alex Xu]

Attachment: fix-clang-build-RestyleManager.patch

Works for me.

Comment 3

[Mats Palmgren (inactive)]

Comment on attachment 784129 [details] [diff] [review]
fix-clang-build-RestyleManager.patch

Grouping the ACCESSIBILITY members in one block is good, but please move
the mVisibleKidsOfHiddenElement field after mResolvedChild so we don't
waste space due to alignment requirements (don't forget to move it in
the ctor initializers as well).

Also, can we move mDesiredA11yNotifications and
mKidsDesiredA11yNotifications inside #ifdef ACCESSIBILITY as well?
It looks like it to me.

Comment 4

[David Baron :dbaron:]

Comment on attachment 784129 [details] [diff] [review]
fix-clang-build-RestyleManager.patch

If there are any #ifdefs here, they should cover all 5 variables in that block (which should all be touched only #ifdef ACCESSIBILITY).

Comment 5

[Alex Xu]

Attachment: fix-clang-build-RestyleManager.patch

Comment 6

[David Baron :dbaron:]

Comment on attachment 784151 [details] [diff] [review]
fix-clang-build-RestyleManager.patch

r=dbaron

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 784151 [details] [diff] [review]
fix-clang-build-RestyleManager.patch

Minor nit: I would still prefer to have mVisibleKidsOfHiddenElement first
in that group of members to avoid the alignment gap.  Not super-important
though - we don't create many of these objects I think.

Comment 8

[David Baron :dbaron:]

Committed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c46dfdd8d90c

In the future, you might want to see:
https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in
and maybe also:
https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch

And thanks for the patch.

Comment 9

[Alex Xu]

I know how to create an hg patch (sorta, see bug 853208 comment 14), but I'm using https://github.com/mozilla/mozilla-central since I'm more familiar with Git.

Is there an easy way to get git format-patch or a similar tool to output hg-format patches? A quick google doesn't seem to turn up anything useful.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Daniel Holbert [:dholbert]]

I(In reply to Alex Xu from comment #9)
> Is there an easy way to get git format-patch or a similar tool to output
> hg-format patches?

You don't need the patches to be in any special hg format; I think you just need "From" and "Subject" lines at the top. (which git format-patch will put in, given the right command-line arguments)

http://mschranz.wordpress.com/2012/01/12/mozilla-bug-process-using-git/ has a git alias that seems to format things correctly, according to the author.

Comment 12

[Alex Xu]

(In reply to Daniel Holbert [:dholbert] from comment #11)
> I(In reply to Alex Xu from comment #9)
> > Is there an easy way to get git format-patch or a similar tool to output
> > hg-format patches?
> 
> You don't need the patches to be in any special hg format; I think you just
> need "From" and "Subject" lines at the top. (which git format-patch will put
> in, given the right command-line arguments)
> 
> http://mschranz.wordpress.com/2012/01/12/mozilla-bug-process-using-git/ has
> a git alias that seems to format things correctly, according to the author.

I recall reading somewhere that there are some minor format differences between the formats; something about one accepting new lines in author names or something vaguely along those lines.

https://github.com/mozilla/moz-git-tools/blob/master/git-patch-to-hg-patch does some fiddling with the format.

Comment 13

[Daniel Holbert [:dholbert]]

Hmm, looks like you might be right -- after a bit more searching I found...
 https://wiki.mozilla.org/B2G/Hacking#Generating_checkin-needed_patches
...which does say "hg can't read git's author and commit-message headers" (though I'm not sure if that's still true). (That does link to the moz-git-tools repo that you linked above.)

Anyway, looks like either those instructions or the blog instructions (or a combination) are your best bet. :)