Bug 563195 comment 22 suggested enabling -Wshadow piecemeal in the tree. This patch adds -Wshadow to moz.build files in directories that currently have no -Wshadow warnings.

Comment on attachment 8652176 [details] [diff] [review] add-Wshadow-to-mozbuild-files.patch Review of attachment 8652176 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the unrelated changes left out, and the layout/base/gtest thing fixed. ::: dom/push/moz.build @@ +48,5 @@ > + > +if CONFIG['GNU_CXX']: > + CXXFLAGS += ['-Wshadow'] > + > +FAIL_ON_WARNINGS = True This change is unrelated, and while I was seeing this, I was wondering how many directories have FAIL_ON_WARNINGS on now. And the answer is 486 as of my tree from last week. Whereas a crude git grep -l SOURCES -- '*/moz.build' '*.mozbuild' say there are 653 files with SOURCES in them (which, incidentally, also matches XPIDL_SOURCES, so that would be an upper bound). I think it's well about time to turn the default around for this. Would you mind doing this in a separate bug? ::: layout/base/gtest/moz.build @@ +18,5 @@ > # Workaround bug 1142396. Suppress the warning from gmock library for clang. > if CONFIG['CLANG_CXX']: > + CXXFLAGS += [ > + '-Wno-null-dereference', > + '-Wshadow', That's clang specific branch, that leaves out GCC. ::: memory/replace/jemalloc/moz.build @@ +3,5 @@ > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +UNIFIED_SOURCES += [ This is unrelated. @@ +12,5 @@ > # Android doesn't have pthread_atfork, so just implement a dummy function. > # It shouldn't make much problem, as the use of fork is pretty limited on > # Android. > if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'android': > + UNIFIED_SOURCES += [ Likewise.

Thanks! (In reply to Mike Hommey [:glandium] from comment #1) > This change is unrelated, and while I was seeing this, I was wondering how > many directories have FAIL_ON_WARNINGS on now. And the answer is 486 as of > my tree from last week. Whereas a crude git grep -l SOURCES -- '*/moz.build' > '*.mozbuild' say there are 653 files with SOURCES in them (which, > incidentally, also matches XPIDL_SOURCES, so that would be an upper bound). > I think it's well about time to turn the default around for this. Would you > mind doing this in a separate bug? I filed bug 1198334 to change FAIL_ON_WARNINGS' default value from False to True and fix the mozbuild files.

> https://hg.mozilla.org/mozilla-central/rev/fc23dad2b383 The patch modified only 36 directories. Are there really that few that are shadow-free? All the more reason to start pushing that number up, I guess...

(In reply to Nicholas Nethercote [:njn] from comment #5) > The patch modified only 36 directories. Are there really that few that are > shadow-free? All the more reason to start pushing that number up, I guess... Yes, but many (most?) of the warnings are from shadow warnings from ipc/chromium/src/base headers indirectly included in .cpp files. Perhaps we can quarantine the chromium headers in a .cpp file and just expose a Mozilla-style API. But there might be easier wins from some header file trimming. Here is an example -Wshadow warning in widget/gtk from (FFOS's?) NuwaParent.h. From a cursory look at dom/ipc/NuwaParent.h, the header includes chromium's message_loop.h, but AFAICT doesn't actually use any of its declarations! 17:39:25 INFO - In file included from /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/task.h:11:0, 17:39:25 INFO - from /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/message_loop.h:17, 17:39:25 INFO - from ../../dist/include/mozilla/dom/NuwaParent.h:10, 17:39:25 INFO - from ../../dist/include/mozilla/dom/ContentParent.h:10, 17:39:25 INFO - from /builds/slave/try-l64-d-00000000000000000000/build/src/widget/nsBaseDragService.h:18, 17:39:25 INFO - from /builds/slave/try-l64-d-00000000000000000000/build/src/widget/gtk/nsDragService.h:11, 17:39:25 INFO - from /builds/slave/try-l64-d-00000000000000000000/build/src/widget/gtk/nsDragService.cpp:7, 17:39:25 INFO - from /builds/slave/try-l64-d-00000000000000000000/build/src/obj-firefox/widget/gtk/Unified_cpp_widget_gtk0.cpp:101: 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple1<A>::Tuple1(typename TupleTraits<A>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple1<A>::Tuple1(typename TupleTraits<A>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:81:57: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple2<A, B>::Tuple2(typename TupleTraits<A>::ParamType, typename TupleTraits<B>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:99:7: error: declaration of 'b' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:99:7: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple3<A, B, C>::Tuple3(typename TupleTraits<A>::ParamType, typename TupleTraits<B>::ParamType, typename TupleTraits<C>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:123:7: error: declaration of 'c' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:123:7: error: declaration of 'b' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:123:7: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple4<A, B, C, D>::Tuple4(typename TupleTraits<A>::ParamType, typename TupleTraits<B>::ParamType, typename TupleTraits<C>::ParamType, typename TupleTraits<D>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:152:7: error: declaration of 'd' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:152:7: error: declaration of 'c' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:152:7: error: declaration of 'b' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:152:7: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple5<A, B, C, D, E>::Tuple5(typename TupleTraits<A>::ParamType, typename TupleTraits<B>::ParamType, typename TupleTraits<C>::ParamType, typename TupleTraits<D>::ParamType, typename TupleTraits<E>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:186:5: error: declaration of 'e' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:186:5: error: declaration of 'd' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:186:5: error: declaration of 'c' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:186:5: error: declaration of 'b' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:186:5: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple6<A, B, C, D, E, F>::Tuple6(typename TupleTraits<A>::ParamType, typename TupleTraits<B>::ParamType, typename TupleTraits<C>::ParamType, typename TupleTraits<D>::ParamType, typename TupleTraits<E>::ParamType, typename TupleTraits<F>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:225:5: error: declaration of 'f' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:225:5: error: declaration of 'e' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:225:5: error: declaration of 'd' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:225:5: error: declaration of 'c' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:225:5: error: declaration of 'b' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:225:5: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h: In constructor 'Tuple7<A, B, C, D, E, F, G>::Tuple7(typename TupleTraits<A>::ParamType, typename TupleTraits<B>::ParamType, typename TupleTraits<C>::ParamType, typename TupleTraits<D>::ParamType, typename TupleTraits<E>::ParamType, typename TupleTraits<F>::ParamType, typename TupleTraits<G>::ParamType)': 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'g' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'f' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'e' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'd' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'c' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'b' shadows a member of 'this' [-Werror=shadow] 17:39:25 INFO - /builds/slave/try-l64-d-00000000000000000000/build/src/ipc/chromium/src/base/tuple.h:269:5: error: declaration of 'a' shadows a member of 'this' [-Werror=shadow]