Closed Bug 900284 Opened 6 years ago Closed 6 years ago
Build failure with Clang 3
.3 + --enable-warnings-as-errors: private fields 'm Our A11y Notification', 'm Was Frame Visible' are not used
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: *** [RestyleManager.o] Error 1 41:44.31 gmake: *** [base_libs] Error 2 41:44.31 gmake: *** 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.
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
Works for me.
Attachment #784129 - Flags: review?(dbaron)
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 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).
Attachment #784129 - Flags: review?(dbaron) → review-
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 784151 [details] [diff] [review] fix-clang-build-RestyleManager.patch r=dbaron
Attachment #784151 - Flags: review?(dbaron) → review+
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.
Attachment #784151 - Flags: feedback?(matspal) → feedback+
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.
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.
Assignee: nobody → alex_y_xu
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
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.
(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.
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. :)
You need to log in before you can comment on or make changes to this bug.