Bug 22939

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
have:

AC_ARG_ENABLE(strict-compilation, ....)
if test "x$STRICT_COMPILE" = "xyes"; then
    # Set C standard to C99 because that is closest standard to what code uses
    AC_PROG_CC_C99
    if test "x$GCC" = xyes; then
       STRICT_CFLAGS="$CFLAGS -pedantic -Werror"
    elif test "x$SUNCC" = "xyes"; then
        CWARNFLAGS="-errwarn"
    fi
fi

(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.

http://lists.x.org/archives/xorg-devel/2011-November/026780.html