Closed Bug 900284 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Layout, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: alex_y_xu, Assigned: alex_y_xu)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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-
Attachment #784129 - Attachment is obsolete: true
Attachment #784151 - Flags: review?(dbaron)
Attachment #784151 - Flags: feedback?(matspal)
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+
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.
https://hg.mozilla.org/mozilla-central/rev/c46dfdd8d90c
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.