Re: [webkit-dev] Let's use -Werror on EWS?
On Tue, May 25 2021 at 06:22:41 AM -0500, Michael Catanzaro via webkit-dev wrote: I'm hoping there are not very many warnings, since I've been cleaning warnings I see for several years now. There will probably be a few, though, which could be caused by (a) EWS using non-default build options like -DENABLE_EXPERIMENTAL_FEATURES=ON, which notably enables building WebRTC, and (b) using older GCC versions or other older dependencies than I do. So a few extra warnings are likely simply because my personal build environment is not identical to EWS. I forgot about builds on 32-bit architectures, which are filled with pointer aliasing warning spam reminding us how unsafe our code is. We don't have any EWS for those platforms, though, only regular bots (which, again, should definitely not use -Werror, because we don't want to lose a night of test results to a silly build warning). ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Let's use -Werror on EWS?
On Mon, May 24 2021 at 07:36:03 PM -0700, Darin Adler via webkit-dev wrote: I do not know why we do not already use -Werror on GTK and WPE and I support using it there after fixing all the warnings. I'd be willing to enable it at the CMake level if it was conditional on -DENABLE_DEVELOPER_MODE=ON. I tried proposing that previously in https://bugs.webkit.org/show_bug.cgi?id=155047 but it was not approved at the time. Of course we can't enable -Werror by default, though, since it would be offensive to distributors and non-WebKit developers trying to build WebKit. And it would make bisecting pretty hard for ourselves too. We don't want non-EWS builds to fail just because you're using a newer compiler or a different optimization level that causes warnings to be more sensitive. So it should only be used on EWS or in DEVELOPER_MODE. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Let's use -Werror on EWS?
On Mon, May 24 2021 at 06:32:03 PM -0700, Darin Adler via webkit-dev wrote: But we can’t just turn on -Werror until after we fix all the warnings! Who will do that project. I'm hoping there are not very many warnings, since I've been cleaning warnings I see for several years now. There will probably be a few, though, which could be caused by (a) EWS using non-default build options like -DENABLE_EXPERIMENTAL_FEATURES=ON, which notably enables building WebRTC, and (b) using older GCC versions or other older dependencies than I do. So a few extra warnings are likely simply because my personal build environment is not identical to EWS. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Let's use -Werror on EWS?
Sorry, I should clarify. Apple’s ports of WebKit already use -Werror and always have, everywhere, not just on EWS. Since day one. I do not know why we do not already use -Werror on GTK and WPE and I support using it there after fixing all the warnings. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Let's use -Werror on EWS?
I’m a big fan of -Werror. Back when WebKit started, it was controversial to use it at Apple. But we can’t just turn on -Werror until after we fix all the warnings! Who will do that project. — Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Let's use -Werror on EWS?
On Mon, May 24 2021 at 05:42:37 PM -0500, Michael Catanzaro wrote: But really, rather than cherry-picking particular warning flags, using -Werror seems simplest to me. Problematic warnings should be disabled or suppressed. We might want to globally suppress -Warray-bounds and -Wnonnull when compiling with GCC (not with Clang) since GCC has frankly become pretty bad with these and they're almost always false-positives. It's a shame, because these warnings sometimes do catch serious bugs, but relying on Clang developers to notice these might be sufficient. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Let's use -Werror on EWS?
Hi, I'd like to suggest turning on -Werror on at least the GTK and WPE EWS, to reduce the amount of time I spend cleaning up newly-introduced build warnings. ;) If we're worried about too many spurious build failures, let's at least build with a few flags to prevent the most common GCC warnings that developers using Clang will never notice: -Werror=return-type, -Werror=class-memaccess, and -Werror=pessimizing-move. These are simple to avoid if you see the warning from GCC, but very easy to miss if you develop with Clang. I guess these account for more than half of new GCC warnings introduced into WebKit. I'd also like to see -Werror=unused, since it's easy to introduce these warnings for other ports when doing anything involving conditional compilation. That might require some CMake changes, though (as we don't want to break -Wno-unused-parameter, which we use when building Source/WebKit and several directories under Tools). But really, rather than cherry-picking particular warning flags, using -Werror seems simplest to me. Problematic warnings should be disabled or suppressed. I do *not* suggest using -Werror on any non-EWS bots, since that will make gardening harder for almost no benefit. We do not want to lose test results due to a missing UNUSED_PARAM() or RELEASE_ASSERT_NOT_REACHED() somewhere. I also certainly do not suggest using it by default in CMake, which would really annoy our distributors. I would only use it in the EWS bot config. Michael ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
