Re: [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror

[Thomas Huth]

On Mon, 24 Feb 2014 19:08:18 +0000
Peter Maydell <address@hidden> wrote:

> gcc's C++ compiler complains about being passed some -W options
> which make sense for C but not for C++. This means we mustn't try
> a C++ compile with QEMU_CFLAGS, but only with a filtered version
> that removes the offending options. This filtering was already being
> done for uses of C++ in the build itself, but was omitted for the
> "does C++ work?" configure test. This only showed up when doing
> builds which explicitly enabled -Werror with --enable-werror,
> because the "do the compilers work" tests were mistakenly placed
> above the "default werror based on whether compiling from git" code.
> Further, when the test did fail configure would plunge on regardless
> of the error since we were running do_cc in a subshell. Fix this
> complex of errors:
> 
> 1. Move the default-werror code up so that there are no invocations
> of compile_object and friends between it and the point where we
> set $werror explicitly based on the --enable-werror command line
> option.
> 
> 2. Provide a mechanism for filtering QEMU_CFLAGS to create
> QEMU_CXXFLAGS, and use it for the test we run here.
> 
> 3. Provide a do_cxx function to run a test with the C++ compiler
> rather than doing cute tricks with subshells and do_cc.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> This was pretty confusing to debug (mostly because I got tripped
> up by the issue fixed by 1. above)...
> 
> We could probably roll do_libtool into do_compiler, but I'd rather
> not mix that cleanup up with this build-breakage fix.
> 
>  configure | 70 
> +++++++++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 50 insertions(+), 20 deletions(-)
> 
> diff --git a/configure b/configure
> index 00f9070..d5929d6 100755
> --- a/configure
> +++ b/configure
> @@ -54,10 +54,13 @@ error_exit() {
>      exit 1
>  }
> 
> -do_cc() {
> -    # Run the compiler, capturing its output to the log.
> -    echo $cc "$@" >> config.log
> -    $cc "$@" >> config.log 2>&1 || return $?
> +do_compiler() {
> +    # Run the compiler, capturing its output to the log. First argument
> +    # is compiler binary to execute.
> +    local compiler="$1"
> +    shift
> +    echo $compiler "$@" >> config.log
> +    $compiler "$@" >> config.log 2>&1 || return $?
>      # Test passed. If this is an --enable-werror build, rerun
>      # the test with -Werror and bail out if it fails. This
>      # makes warning-generating-errors in configure test code
> @@ -71,14 +74,39 @@ do_cc() {
>             return 0
>          ;;
>      esac
> -    echo $cc -Werror "$@" >> config.log
> -    $cc -Werror "$@" >> config.log 2>&1 && return $?
> +    echo $compiler -Werror "$@" >> config.log
> +    $compiler -Werror "$@" >> config.log 2>&1 && return $?
>      error_exit "configure test passed without -Werror but failed with 
> -Werror." \
>          "This is probably a bug in the configure script. The failing 
> command" \
>          "will be at the bottom of config.log." \
>          "You can run configure with --disable-werror to bypass this check."
>  }
> 
> +do_cc() {
> +    do_compiler "$cc" "$@"
> +}
> +
> +do_cxx() {
> +    do_compiler "$cxx" "$@"
> +}
> +
> +update_cxxflags() {
> +    # Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those
> +    # options which some versions of GCC's C++ compiler complain about
> +    # because they only make sense for C programs.
> +    QEMU_CXXFLAGS=
> +    for arg in $QEMU_CFLAGS; do
> +        case $arg in
> +            -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
> +            -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls)
> +                ;;
> +            *)
> +                QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg
> +                ;;
> +        esac
> +    done
> +}
> +
>  compile_object() {
>    do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
>  }
> @@ -1320,6 +1348,19 @@ if test "$ARCH" = "unknown"; then
>      fi
>  fi
> 
> +# Consult white-list to determine whether to enable werror
> +# by default.  Only enable by default for git builds
> +z_version=`cut -f3 -d. $source_path/VERSION`
> +
> +if test -z "$werror" ; then
> +    if test -d "$source_path/.git" -a \
> +        "$linux" = "yes" ; then
> +        werror="yes"
> +    else
> +        werror="no"
> +    fi
> +fi
> +
>  # check that the C compiler works.
>  cat > $TMPC <<EOF
>  int main(void) { return 0; }
> @@ -1347,7 +1388,9 @@ extern "C" {
>  int c_function(void) { return 42; }
>  EOF
> 
> -    if (cc=$cxx do_cc $QEMU_CFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS); then
> +    update_cxxflags
> +
> +    if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS; then
>          # C++ compiler $cxx works ok with C compiler $cc
>          :
>      else
> @@ -1360,19 +1403,6 @@ else
>      cxx=
>  fi
> 
> -# Consult white-list to determine whether to enable werror
> -# by default.  Only enable by default for git builds
> -z_version=`cut -f3 -d. $source_path/VERSION`
> -
> -if test -z "$werror" ; then
> -    if test -d "$source_path/.git" -a \
> -        "$linux" = "yes" ; then
> -        werror="yes"
> -    else
> -        werror="no"
> -    fi
> -fi
> -
>  gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
>  gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
> $gcc_flags"
>  gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"

Looks fine to me.
Reviewed-by: Thomas Huth <address@hidden>