Bugzilla – Bug 22939
[PATCH] Add strict compilation option to util/macros
Last modified: 2011-11-07 14:30:28 UTC
Created attachment 27987 [details] [review]
Patch to add --enable-strict-compilation
This is idea how to make it easy for all components to add --enable-strict-compilation configure option to help developers find bugs from code early in development.
There is a few additions that I though might be good idea but didn't want to add without comments.
I thought about making the macro automatically add flags to CPPFLAGS but I'm not sure if it is good idea.
Possible addition to macro is detection if we are compiling from git from not tagged version. In that case default could be enabling strict compilation while keeping default disabled for tagged/release versions.
Also flags for sun or intel compilers would be nice (if they are supported build targets).
This is an interesting idea - certainly we'd have to discuss on xorg-devel
before considering making strict the default in development/git builds.
As someone who compiles regularly with the Sun compilers, I can vouch for them
being a supported target. To enable support of multiple compilers, I'd swap
the order of the tests, making the gcc test the inner test, so you'd instead
if test "x$STRICT_COMPILE" = "xyes"; then
# Set C standard to C99 because that is closest standard to what code uses
if test "x$GCC" = xyes; then
STRICT_CFLAGS="$CFLAGS -pedantic -Werror"
elif test "x$SUNCC" = "xyes"; then
(You can see how to test for Sun cc and set SUNCC in the current
XORG_CWARNFLAGS macro. -errwarn is the Sun compiler flag to turn
warnings into errors - if you want to see documentation of the other
available options, see http://docs.sun.com/app/docs/doc/819-5265 .)
I also wonder if AC_PROG_CC_C99 should be always enabled, not just in
strict compilation mode, since that may cause strange, hard to debug
changes between strict and non-strict modes.
Also, I should probably document that the intended versioning scheme for
xorg-macros is to bump the minor version when adding new macros, so this
would have Minimum version 1.3.0 (which is why the XORG_MACROS_VERSION
only checks major & minor versions, since micro versions are just bug fixes
and implementation details).
Created attachment 27988 [details] [review]
[PATCH 1/4] Add XORG_STRICT_OPTION macro for strict compilation option
Updated version of patch (multiple patches this time)
Created attachment 27989 [details] [review]
[PATCH 2/4] Add intel compiler support to XORG_CWARNFLAGS.
Created attachment 27990 [details] [review]
[PATCH 3/4] Remove -fbad-function-cast and add -fstrict-aliasing=2.
Created attachment 27991 [details] [review]
[PATCH 4/4] Version bump: 1.3.0
Created attachment 27992 [details] [review]
Example how to add XORG_STRICT_OPTION to libdrm
Created attachment 28003 [details] [review]
[PATCH 3/4] Remove -fbad-function-cast and add -wstrict-aliasing=2.
fixed the error in compiler switch
Patch 1/4 (XORG_STRICT_OPTION) pushed as 3b7dd69d0bf6bc19f0e4403bb6611de87497aac3
xorg-macros 1.3.0 has been released with these, so it can be used for the
upcoming X11R7.5 RC's. The additional patches to tweak the flags for
specific compilers can go in a later 1.3.x release after people who use those
compilers can confirm they're the right ones - since they don't change the
macro definitions, just the implementations, we didn't think it was necessary
to hold 1.3.0 for them.
Thanks for submitting these.
Should this bug report considered fixed?
No, CWARNFLAGS still has fno-strict-aliasing in it.
We've been building without -fno-strict-aliasing (aside from xorg-server) in
XQuartz for a long time now without issue. I really think we should remove
-fno-strict-aliasing from CWARNFLAGS and include it in xorg-server/configure.ac
... is the only thing holding that up the fear of using the newr util-macros on
an older server?
(In reply to comment #12)
> We've been building without -fno-strict-aliasing (aside from xorg-server) in
> XQuartz for a long time now without issue. I really think we should remove
> -fno-strict-aliasing from CWARNFLAGS and include it in xorg-server/configure.ac
> ... is the only thing holding that up the fear of using the newr util-macros on
> an older server?
Yes, that is the "fear" a.k.a. backward compatibility. For the server and the libs and the drivers...
A simple solution is to leave CWARNFLAGS alone and create a new warning variable without the offending flag. Each module then decide which one to use. That's the usual strategy, but yes, dual maintenance for the macros. Still the lowest cost solution.
Ok, I'll come up with something "soon" then, so we can do this before the next katamari
An interim solution for the server would be to simply append after CWARNFLAGS in configure.ac appropriate counter-flags.
# Quoted so that make will expand $(CWARNFLAGS) in makefiles to allow
# easier overrides at build time.
XSERVER_CFLAGS='$(CWARNFLAGS)' +++ add counter-flags +++
This has the merit of being backward compatible and not requiring any util-macros change. It also limits the discussion to the xserver and reduce complexity for testing.
Once the dust has settled, a solution in util-macros can be devised which will be transparent to the server.
Yeah, I have that patch already, since I'm using it in XQuartz. I just sent it to the list for review.
I sent a series to xorg-devel to address this. I need help with the recursion in patch 2. m4 doesn't like me.