re: configure.ac: add -Werror to default compiler options
(Reviving http://www.winehq.org/pipermail/wine-devel/2008-September/069266.html ) After http://www.winehq.org/pipermail/wine-patches/2011-August/106150.html and http://www.winehq.org/pipermail/wine-patches/2011-August/106151.html, configuring with -Wall -Werror gives the right results, judging by a quick diff of config.h and config.status. (sys/asoundlib.h is not detected, but that's ok, it's marked as obsolete, and wine uses alsa/asoundlib.h instead.) This takes care of Maarten's concerns from the earlier thread. Then building yields only a tiny handful of errors: dllfunc.c: In function ‘AMovieDllRegisterServer2’: dllfunc.c:180: warning: dereferencing type-punned pointer will break strict-aliasing rules qualitycontrol.c: In function ‘QualityControlImpl_Notify’: qualitycontrol.c:70: warning: dereferencing type-punned pointer will break strict-aliasing rules transform.c: In function ‘TransformFilterImpl_QueryInterface’: transform.c:248: warning: dereferencing type-punned pointer will break strict-aliasing rules ../../include/winternl.h: In function ‘get_vm86_teb_info’: ../../include/winternl.h:2660: warning: dereferencing type-punned pointer will break strict-aliasing rules ... register.c: In function ‘create_registrar’: register.c:66: warning: dereferencing type-punned pointer will break strict-aliasing rules Those are potential, if unlikely, bugs; we ought to either fix them or add -fno-strict-aliasing to CFLAGS for the seven or so affected .c files. At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses. - Dan
On Thu, Sep 1, 2011 at 4:27 AM, Dan Kegel <dank(a)kegel.com> wrote:
At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses.
Have you tried compiling on 64-bit? There are still a handful of compile warnings there (most in oleaut32/tmarshal.c; then setupapi/devinst.c; dsound+winmm). Octavian
Here is a warning report from a 64bit system. I get a lot of warnings on both 32 and 64bit wines. Many of them are "variable set but not used" (unused error returns, ...). http://bugs.winehq.org/show_bug.cgi?id=26768 was filed a while ago for the oleaut32 spam. On Thu, Sep 1, 2011 at 7:13 AM, Octavian Voicu <octavian.voicu(a)gmail.com> wrote:
On Thu, Sep 1, 2011 at 4:27 AM, Dan Kegel <dank(a)kegel.com> wrote:
At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses.
Have you tried compiling on 64-bit? There are still a handful of compile warnings there (most in oleaut32/tmarshal.c; then setupapi/devinst.c; dsound+winmm).
Octavian
On Thu, Sep 1, 2011 at 1:35 AM, Jerome Leclanche <adys.wh(a)gmail.com> wrote:
Here is a warning report from a 64bit system. I get a lot of warnings on both 32 and 64bit wines. Many of them are "variable set but not used" (unused error returns, ...).
http://bugs.winehq.org/show_bug.cgi?id=26768 was filed a while ago for the oleaut32 spam.
Wow. Guess we have a ways to go. - Dan
Am 01.09.2011 17:52, schrieb Dan Kegel:
On Thu, Sep 1, 2011 at 1:35 AM, Jerome Leclanche <adys.wh(a)gmail.com> wrote:
Here is a warning report from a 64bit system. I get a lot of warnings on both 32 and 64bit wines. Many of them are "variable set but not used" (unused error returns, ...).
http://bugs.winehq.org/show_bug.cgi?id=26768 was filed a while ago for the oleaut32 spam.
Wow. Guess we have a ways to go. - Dan
Also beside Micheal's comment -Werror is a bad idea for crosscompiling, compiling with other compilers, compiling on new/uncommon systems, compiling on other architectures and so on... -- Best Regards, André Hentschel
On Thu, Sep 1, 2011 at 8:52 AM, Dan Kegel <dank(a)kegel.com> wrote:
On Thu, Sep 1, 2011 at 1:35 AM, Jerome Leclanche <adys.wh(a)gmail.com> wrote:
Here is a warning report from a 64bit system. I get a lot of warnings on both 32 and 64bit wines. Many of them are "variable set but not used" (unused error returns, ...).
http://bugs.winehq.org/show_bug.cgi?id=26768 was filed a while ago for the oleaut32 spam.
Wow. Guess we have a ways to go.
Finally dug up AJ's position on the question: http://www.winehq.org/pipermail/wine-devel/2008-September/069203.html So, no -Werror by default in git. Buildbot may at some point start complaining about patches that introduce new warnings, though. Dan
On Wed, Aug 31, 2011 at 11:13 PM, Octavian Voicu <octavian.voicu(a)gmail.com> wrote:
On Thu, Sep 1, 2011 at 4:27 AM, Dan Kegel <dank(a)kegel.com> wrote:
At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses.
Have you tried compiling on 64-bit? There are still a handful of compile warnings there (most in oleaut32/tmarshal.c; then setupapi/devinst.c; dsound+winmm).
OK, finally tried this myself. It looks like oleaut32/tmarshal.c is the only serious obstacle left. - Dan
1 sep 2011 kl. 03:27 skrev Dan Kegel:
At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses.
We use -Werror at my company, but I think you should really think twice before enabling it for a distributed package. New compiler versions add new warnings. Usually correctly so, but there's no way to know what warnings will be produced by what compiler, and keeping up with new compilers requires a bit of effort. We make sure to always test new compilers separately before upgrading on the developing machines. I think most users will probably just see that the package fails to build, and assume it's broken. Ps. I know there's also a few warnings while building on OS X. I don't have access to it now, or I'd send the list. -- Pelle Johansson
Dan Kegel wrote:
(Reviving http://www.winehq.org/pipermail/wine-devel/2008-September/069266.html )
After http://www.winehq.org/pipermail/wine-patches/2011-August/106150.html and http://www.winehq.org/pipermail/wine-patches/2011-August/106151.html, configuring with -Wall -Werror gives the right results, judging by a quick diff of config.h and config.status.
(sys/asoundlib.h is not detected, but that's ok, it's marked as obsolete, and wine uses alsa/asoundlib.h instead.)
This takes care of Maarten's concerns from the earlier thread.
Then building yields only a tiny handful of errors:
dllfunc.c: In function ‘AMovieDllRegisterServer2’: dllfunc.c:180: warning: dereferencing type-punned pointer will break strict-aliasing rules qualitycontrol.c: In function ‘QualityControlImpl_Notify’: qualitycontrol.c:70: warning: dereferencing type-punned pointer will break strict-aliasing rules transform.c: In function ‘TransformFilterImpl_QueryInterface’: transform.c:248: warning: dereferencing type-punned pointer will break strict-aliasing rules ../../include/winternl.h: In function ‘get_vm86_teb_info’: ../../include/winternl.h:2660: warning: dereferencing type-punned pointer will break strict-aliasing rules ... register.c: In function ‘create_registrar’: register.c:66: warning: dereferencing type-punned pointer will break strict-aliasing rules
Those are potential, if unlikely, bugs; we ought to either fix them or add -fno-strict-aliasing to CFLAGS for the seven or so affected .c files.
At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses. You can try submitting a patch to add -Werror by default but I already know the patch status that it will get: REJECTED!!11!! You might get away with enabling it for the buildbot as you can enforce a controlled environment. But enabling it in general is such a bad idea that it isn't funny. No clue why people insist to still do it. Ask packagers what they think about upstreams that do that, disabling that is one of the first actions they do.
bye michael
(resent with correct from-address, please ignore the previous one) 1 sep 2011 kl. 03:27 skrev Dan Kegel:
At which point it would probably be a fine idea to add -Werror by default; buildbot will help keep everyone in sync, even if they're using a compiler that doesn't catch as many warnings as the one buildbot uses.
We use -Werror at my company, but I think you should really think twice before enabling it for a distributed package. New compiler versions add new warnings. Usually correctly so, but there's no way to know what warnings will be produced by what compiler, and keeping up with new compilers requires a bit of effort. We make sure to always test new compilers separately before upgrading on the developing machines. I think most users will probably just see that the package fails to build, and assume it's broken. Ps. I know there's also a few warnings while building on OS X. I don't have access to it now, or I'd send the list. -- Per Johansson
participants (7)
-
André Hentschel -
Dan Kegel -
Jerome Leclanche -
Michael Stefaniuc -
Octavian Voicu -
Per Johansson -
Per Johansson