From ad4e7af759d7e86a79508525949b6fabef53753f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 May 2011 18:32:51 +0100 Subject: [PATCH 1/6] Import tp-compiler-flag.m4 and tp-compiler-warnings.m4 from telepathy-glib Bug: https://bugs.freedesktop.org/show_bug.cgi?id=19681 Reviewed-by: Colin Walters --- m4/tp-compiler-flag.m4 | 43 +++++++++++++++++++++++++++++++++ m4/tp-compiler-warnings.m4 | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 m4/tp-compiler-flag.m4 create mode 100644 m4/tp-compiler-warnings.m4 diff --git a/m4/tp-compiler-flag.m4 b/m4/tp-compiler-flag.m4 new file mode 100644 index 00000000..06deaba0 --- /dev/null +++ b/m4/tp-compiler-flag.m4 @@ -0,0 +1,43 @@ +dnl A version of AS_COMPILER_FLAG that supports both C and C++. +dnl Based on: + +dnl as-compiler-flag.m4 0.1.0 +dnl autostars m4 macro for detection of compiler flags +dnl David Schleef +dnl $Id: as-compiler-flag.m4,v 1.1 2005/06/18 18:02:46 burgerman Exp $ + +dnl TP_COMPILER_FLAG(CFLAGS, ACTION-IF-ACCEPTED, [ACTION-IF-NOT-ACCEPTED]) +dnl Tries to compile with the given CFLAGS and CXXFLAGS. +dnl +dnl Runs ACTION-IF-ACCEPTED if the compiler for the currently selected +dnl AC_LANG can compile with the flags, and ACTION-IF-NOT-ACCEPTED otherwise. + +AC_DEFUN([TP_COMPILER_FLAG], +[ + AC_MSG_CHECKING([to see if compiler understands $1]) + + save_CFLAGS="$CFLAGS" + save_CXXFLAGS="$CXXFLAGS" + CFLAGS="$CFLAGS $1" + CXXFLAGS="$CXXFLAGS $1" + + AC_TRY_COMPILE([ ], [], [flag_ok=yes], [flag_ok=no]) + CFLAGS="$save_CFLAGS" + CXXFLAGS="$save_CXXFLAGS" + + if test "X$flag_ok" = Xyes ; then + $2 + true + else + $3 + true + fi + AC_MSG_RESULT([$flag_ok]) +]) + +dnl TP_ADD_COMPILER_FLAG(VARIABLE, CFLAGS) +dnl Append CFLAGS to VARIABLE if the compiler supports them. +AC_DEFUN([TP_ADD_COMPILER_FLAG], +[ + TP_COMPILER_FLAG([$2], [$1="[$]$1 $2"]) +]) diff --git a/m4/tp-compiler-warnings.m4 b/m4/tp-compiler-warnings.m4 new file mode 100644 index 00000000..ee4af310 --- /dev/null +++ b/m4/tp-compiler-warnings.m4 @@ -0,0 +1,49 @@ +dnl TP_COMPILER_WARNINGS(VARIABLE, WERROR_BY_DEFAULT, DESIRABLE, UNDESIRABLE) +dnl $1 (VARIABLE): the variable to put flags into +dnl $2 (WERROR_BY_DEFAULT): a command returning true if -Werror should be the +dnl default +dnl $3 (DESIRABLE): warning flags we want (e.g. all extra shadow) +dnl $4 (UNDESIRABLE): warning flags we don't want (e.g. +dnl missing-field-initializers unused-parameter) +AC_DEFUN([TP_COMPILER_WARNINGS], +[ + AC_REQUIRE([AC_ARG_ENABLE])dnl + AC_REQUIRE([AC_HELP_STRING])dnl + AC_REQUIRE([TP_COMPILER_FLAG])dnl + + tp_warnings="" + for tp_flag in $3; do + TP_COMPILER_FLAG([-W$tp_flag], [tp_warnings="$tp_warnings -W$tp_flag"]) + done + + tp_error_flags="-Werror" + TP_COMPILER_FLAG([-Werror], [tp_werror=yes], [tp_werror=no]) + + for tp_flag in $4; do + TP_COMPILER_FLAG([-Wno-$tp_flag], + [tp_warnings="$tp_warnings -Wno-$tp_flag"]) +dnl Yes, we do need to use both -Wno-foo and -Wno-error=foo. Simon says: +dnl some warnings we explicitly don't want, like unused-parameter, but +dnl they're in -Wall. when a distro using cdbs compiles us, we have: +dnl -Werror -Wno-unused-parameter -Wall +dnl ^ from us ^ from cdbs +dnl which turns -Wunused-parameter back on, in effect + TP_COMPILER_FLAG([-Wno-error=$tp_flag], + [tp_error_flags="$tp_error_flags -Wno-error=$tp_flag"], [tp_werror=no]) + done + + AC_ARG_ENABLE([Werror], + AC_HELP_STRING([--disable-Werror], + [compile without -Werror (normally enabled in development builds)]), + tp_werror=$enableval, :) + + if test "x$tp_werror" = xyes && $2; then +dnl We put -Wno-error=foo before -Wno-foo because clang interprets -Wall +dnl -Werror -Wno-foo -Wno-error=foo as “make foo a non-fatal warning”, but does +dnl what we want if you reverse them. + $1="$tp_error_flags $tp_warnings" + else + $1="$tp_warnings" + fi + +]) From e9bf1355d7c16a0ca842dd9b9b17f6ce4a4f863e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 May 2011 19:02:46 +0100 Subject: [PATCH 2/6] Use TP_COMPILER_WARNINGS for all -Wfoo options This consistently checks whether all these options actually work in the current version of gcc. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=19681 Reviewed-by: Colin Walters --- configure.ac | 118 +++++++++++++++------------------------------------ 1 file changed, 34 insertions(+), 84 deletions(-) diff --git a/configure.ac b/configure.ac index 940bfae5..ff6c40a0 100644 --- a/configure.ac +++ b/configure.ac @@ -1117,72 +1117,10 @@ cc_supports_flag() { test "x$rc" = xyes } -# Don't bother with -Werror on Windows for now, too many warnings -if test x$dbus_win != xyes -a x$dbus_cygwin != xyes -a x$USE_MAINTAINER_MODE = xyes; then - if cc_supports_flag "-Werror"; then - CFLAGS="$CFLAGS -Werror" - fi -fi - dnl This whole "if" block is in m4 quotes ([]) because it uses them dnl for character ranges internally. m4 macros cannot be used inside this dnl block. [if test "x$GCC" = "xyes"; then - case " $CFLAGS " in - *[\ \ ]-Wall[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wall" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wchar-subscripts[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wchar-subscripts" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wmissing-declarations[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wmissing-declarations" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wmissing-prototypes[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wmissing-prototypes" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wnested-externs[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wnested-externs" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wpointer-arith[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wpointer-arith" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wcast-align[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wcast-align" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wno-address[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wno-address" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wfloat-equal[\ \ ]*) ;; - *) if cc_supports_flag -Wfloat-equal; then - CFLAGS="$CFLAGS -Wfloat-equal" - fi - ;; - esac - - case " $CFLAGS " in - *[\ \ ]-Wdeclaration-after-statement[\ \ ]*) ;; - *) if cc_supports_flag -Wdeclaration-after-statement; then - CFLAGS="$CFLAGS -Wdeclaration-after-statement" - fi - ;; - esac case " $CFLAGS " in *[\ \ ]-fno-common[\ \ ]*) ;; @@ -1194,28 +1132,6 @@ dnl block. ### Disabled warnings, and compiler flag overrides - # Let's just ignore unused for now - case " $CFLAGS " in - *[\ \ ]-Wno-unused[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wno-unused" ;; - esac - - # This group is for warnings we currently don't pass. - # We would like to, however. Please fix. - - # http://bugs.freedesktop.org/show_bug.cgi?id=17433 - case " $CFLAGS " in - *[\ \ ]-Wno-sign-compare[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wno-sign-compare" ;; - esac - case " $CFLAGS " in - *[\ \ ]-Wno-pointer-sign[\ \ ]*) ;; - *) if cc_supports_flag -Wno-pointer-sign; then - CFLAGS="$CFLAGS -Wno-pointer-sign" - fi - ;; - esac - # This one is special - it's not a warning override. # http://bugs.freedesktop.org/show_bug.cgi?id=10599 case " $CFLAGS " in @@ -1247,6 +1163,40 @@ dnl block. fi fi] +TP_COMPILER_WARNINGS([WARNING_CFLAGS], + dnl Use -Werror by default if: + dnl - we're not on Windows (too many warnings), and + dnl - we're in maintainer mode (a D-Bus developer, not a distro or end-user) + dnl Override with --enable-Werror or --disable-Werror + [test x$dbus_win != xyes -a x$dbus_cygwin != xyes -a x$USE_MAINTAINER_MODE = xyes], + + dnl enable these warnings if possible: + [all \ + char-subscripts \ + missing-declarations \ + missing-prototypes \ + nested-externs \ + pointer-arith \ + cast-align \ + no-address \ + float-equal \ + declaration-after-statement \ + ], + + dnl disable these warnings if possible, make them non-fatal if possible, + dnl and don't enable -Werror unless we succeeded: + dnl (unused is by design, sign-compare and pointer-sign are fd.o #17433) + [unused \ + sign-compare \ + pointer-sign \ + ]) + +dnl In principle we should put WARNING_CFLAGS in each Makefile.am like +dnl telepathy-glib does, since CFLAGS is meant to be reserved for the user... +dnl but prepending to CFLAGS (so the user can override it with later CFLAGS) +dnl is the next best thing +CFLAGS="$WARNING_CFLAGS $CFLAGS" + # Disabling gc-sections makes our binaries bigger, but some toolchains # have known-broken support for it which discards sections that are needed. # See https://bugs.freedesktop.org/show_bug.cgi?id=33466 From 80d08470121a9b96dcc0368e9c55df28269bfcc4 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 May 2011 19:03:12 +0100 Subject: [PATCH 3/6] Use TP_ADD_COMPILER_FLAG to simplify application of warning-like CFLAGS This also means we check for support for them. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=19681 Reviewed-by: Colin Walters --- configure.ac | 60 ++++++++++++---------------------------------------- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/configure.ac b/configure.ac index ff6c40a0..e097718d 100644 --- a/configure.ac +++ b/configure.ac @@ -1117,52 +1117,6 @@ cc_supports_flag() { test "x$rc" = xyes } -dnl This whole "if" block is in m4 quotes ([]) because it uses them -dnl for character ranges internally. m4 macros cannot be used inside this -dnl block. -[if test "x$GCC" = "xyes"; then - - case " $CFLAGS " in - *[\ \ ]-fno-common[\ \ ]*) ;; - *) if cc_supports_flag -fno-common; then - CFLAGS="$CFLAGS -fno-common" - fi - ;; - esac - - ### Disabled warnings, and compiler flag overrides - - # This one is special - it's not a warning override. - # http://bugs.freedesktop.org/show_bug.cgi?id=10599 - case " $CFLAGS " in - *[\ \ ]-fno-strict-aliasing[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -fno-strict-aliasing" ;; - esac - ### End disabled warnings - - if test "x$enable_ansi" = "xyes"; then - case " $CFLAGS " in - *[\ \ ]-ansi[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -ansi" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-D_POSIX_C_SOURCE*) ;; - *) CFLAGS="$CFLAGS -D_POSIX_C_SOURCE=199309L" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-D_BSD_SOURCE[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -D_BSD_SOURCE" ;; - esac - - case " $CFLAGS " in - *[\ \ ]-pedantic[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -pedantic" ;; - esac - fi -fi] - TP_COMPILER_WARNINGS([WARNING_CFLAGS], dnl Use -Werror by default if: dnl - we're not on Windows (too many warnings), and @@ -1191,6 +1145,20 @@ TP_COMPILER_WARNINGS([WARNING_CFLAGS], pointer-sign \ ]) +if test "x$GCC" = "xyes"; then + # We're treating -fno-common like a warning: it makes the linker more + # strict, because on some systems the linker is *always* this strict + TP_ADD_COMPILER_FLAG([WARNING_CFLAGS], [-fno-common]) + + # http://bugs.freedesktop.org/show_bug.cgi?id=10599 + TP_ADD_COMPILER_FLAG([WARNING_CFLAGS], [-fno-strict-aliasing]) + + if test "x$enable_ansi" = "xyes"; then + TP_ADD_COMPILER_FLAG([WARNING_CFLAGS], + [-ansi -D_POSIX_C_SOURCE=199309L -D_BSD_SOURCE -pedantic]) + fi +fi + dnl In principle we should put WARNING_CFLAGS in each Makefile.am like dnl telepathy-glib does, since CFLAGS is meant to be reserved for the user... dnl but prepending to CFLAGS (so the user can override it with later CFLAGS) From 5a665203001d30b3167089fb06af0f1c4a8e1885 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 May 2011 18:48:52 +0100 Subject: [PATCH 4/6] When checking for va_copy, use AC_LANG_SOURCE to shut up recent autoconf Bug: https://bugs.freedesktop.org/show_bug.cgi?id=19681 Reviewed-by: Colin Walters --- configure.ac | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index e097718d..6df79fd6 100644 --- a/configure.ac +++ b/configure.ac @@ -357,7 +357,7 @@ dnl ********************************** dnl we currently check for all three va_copy possibilities, so we get dnl all results in config.log for bug reports. AC_CACHE_CHECK([for an implementation of va_copy()],dbus_cv_va_copy,[ - AC_LINK_IFELSE([#include + AC_LINK_IFELSE([AC_LANG_SOURCE([#include #include static void f (int i, ...) { va_list args1, args2; @@ -370,12 +370,12 @@ AC_CACHE_CHECK([for an implementation of va_copy()],dbus_cv_va_copy,[ int main() { f (0, 42); return 0; - }], + }])], [dbus_cv_va_copy=yes], [dbus_cv_va_copy=no]) ]) AC_CACHE_CHECK([for an implementation of __va_copy()],dbus_cv___va_copy,[ - AC_LINK_IFELSE([#include + AC_LINK_IFELSE([AC_LANG_SOURCE([#include #include static void f (int i, ...) { va_list args1, args2; @@ -388,7 +388,7 @@ AC_CACHE_CHECK([for an implementation of __va_copy()],dbus_cv___va_copy,[ int main() { f (0, 42); return 0; - }], + }])], [dbus_cv___va_copy=yes], [dbus_cv___va_copy=no]) ]) From dfb6affd68df1df8f5890e0d809fe7ac02a4a02c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 May 2011 18:49:54 +0100 Subject: [PATCH 5/6] When checking for __sync_sub_and_fetch, don't underquote, to shut up recent autoconf Without the correct number of levels of quoting, autoconf mistakenly believes we didn't use AC_LANG_SOURCE where required. (In fact, AC_LANG_PROGRAM calls AC_LANG_SOURCE.) Bug: https://bugs.freedesktop.org/show_bug.cgi?id=19681 Reviewed-by: Colin Walters --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 6df79fd6..7001a879 100644 --- a/configure.ac +++ b/configure.ac @@ -441,8 +441,8 @@ fi AC_CACHE_CHECK([whether $CC knows __sync_sub_and_fetch()], dbus_cv_sync_sub_and_fetch, - [AC_LINK_IFELSE( - AC_LANG_PROGRAM([], [[int a = 4; int b = __sync_sub_and_fetch(&a, 4); exit(b); ]]), + [AC_LINK_IFELSE([ + AC_LANG_PROGRAM([[]], [[int a = 4; int b = __sync_sub_and_fetch(&a, 4); exit(b); ]])], [dbus_cv_sync_sub_and_fetch=yes], [dbus_cv_sync_sub_and_fetch=no]) ]) From 0092a75153a0fd9d028aec33ddbef7fd0a4a1f32 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 25 May 2011 17:00:07 +0100 Subject: [PATCH 6/6] NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index c717456b..b86540ae 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,8 @@ Changes: • Clarify documentation (fd.o #35182, Simon McVittie) • Clean up minor dead code and some incorrect error handling (fd.o #33128, fd.o #29881; Simon McVittie) + • Check that compiler options are supported before using them (fd.o #19681, + Simon McVittie) • Windows: • Remove obsolete workaround for winioctl.h (fd.o #35083, Ralf Habacker)