Re: [PATCH 25/30] build: move warning flag selection to meson

[Marc-André Lureau]

Hi

On Fri, Dec 9, 2022 at 3:48 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Meson already knows to test with the positive form of the flag, which
> simplifies the test.  Warnings are now tested explicitly for the C++
> compiler, instead of hardcoding those that are only available for
> the C language.
>
> At this point all compiler flags in QEMU_CFLAGS are global and only
> depend on the OS.  No feature tests are performed in configure.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  configure                | 94 ----------------------------------------
>  contrib/plugins/Makefile |  3 +-
>  meson.build              | 72 ++++++++++++++++++++----------
>  3 files changed, 51 insertions(+), 118 deletions(-)
>
> diff --git a/configure b/configure
> index 5d31294f316f..6df61f4337e4 100755
> --- a/configure
> +++ b/configure
> @@ -75,7 +75,6 @@ fi
>  TMPB="qemu-conf"
>  TMPC="${TMPDIR1}/${TMPB}.c"
>  TMPO="${TMPDIR1}/${TMPB}.o"
> -TMPM="${TMPDIR1}/${TMPB}.m"
>  TMPE="${TMPDIR1}/${TMPB}.exe"
>
>  rm -f config.log
> @@ -157,15 +156,6 @@ do_cc() {
>      do_compiler_werror "$cc" $CPU_CFLAGS "$@"
>  }
>
> -do_objc() {
> -    do_compiler_werror "$objcc" $CPU_CFLAGS "$@"
> -}
> -
> -# Append $2 to the variable named $1, with space separation
> -add_to() {
> -    eval $1=\${$1:+\"\$$1 \"}\$2
> -}
> -
>  compile_object() {
>    local_cflags="$1"
>    do_cc $CFLAGS $EXTRA_CFLAGS $CONFIGURE_CFLAGS $QEMU_CFLAGS $local_cflags 
> -c -o $TMPO $TMPC
> @@ -1091,89 +1081,6 @@ if ! compile_prog "" "" ; then
>      error_exit "You need at least GCC v7.4 or Clang v6.0 (or XCode Clang 
> v10.0)"
>  fi
>
> -# Accumulate -Wfoo and -Wno-bar separately.
> -# We will list all of the enable flags first, and the disable flags second.
> -# Note that we do not add -Werror, because that would enable it for all
> -# configure tests. If a configure test failed due to -Werror this would
> -# just silently disable some features, so it's too error prone.
> -
> -warn_flags=
> -add_to warn_flags -Wundef
> -add_to warn_flags -Wwrite-strings
> -add_to warn_flags -Wmissing-prototypes
> -add_to warn_flags -Wstrict-prototypes
> -add_to warn_flags -Wredundant-decls
> -add_to warn_flags -Wold-style-declaration
> -add_to warn_flags -Wold-style-definition
> -add_to warn_flags -Wtype-limits
> -add_to warn_flags -Wformat-security
> -add_to warn_flags -Wformat-y2k
> -add_to warn_flags -Winit-self
> -add_to warn_flags -Wignored-qualifiers
> -add_to warn_flags -Wempty-body
> -add_to warn_flags -Wnested-externs
> -add_to warn_flags -Wendif-labels
> -add_to warn_flags -Wexpansion-to-defined
> -add_to warn_flags -Wimplicit-fallthrough=2
> -
> -nowarn_flags=
> -add_to nowarn_flags -Wno-initializer-overrides
> -add_to nowarn_flags -Wno-missing-include-dirs
> -add_to nowarn_flags -Wno-shift-negative-value
> -add_to nowarn_flags -Wno-string-plus-int
> -add_to nowarn_flags -Wno-typedef-redefinition
> -add_to nowarn_flags -Wno-tautological-type-limit-compare
> -add_to nowarn_flags -Wno-psabi
> -add_to nowarn_flags -Wno-gnu-variable-sized-type-not-at-end
> -
> -gcc_flags="$warn_flags $nowarn_flags"
> -
> -cc_has_warning_flag() {
> -    write_c_skeleton;
> -
> -    # Use the positive sense of the flag when testing for -Wno-wombat
> -    # support (gcc will happily accept the -Wno- form of unknown
> -    # warning options).
> -    optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
> -    compile_prog "-Werror $optflag" ""
> -}
> -
> -objcc_has_warning_flag() {
> -    cat > $TMPM <<EOF
> -int main(void) { return 0; }
> -EOF
> -
> -    # Use the positive sense of the flag when testing for -Wno-wombat
> -    # support (gcc will happily accept the -Wno- form of unknown
> -    # warning options).
> -    optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
> -    do_objc -Werror $optflag \
> -      $OBJCFLAGS $EXTRA_OBJCFLAGS $CONFIGURE_OBJCFLAGS $QEMU_OBJCFLAGS \
> -      -o $TMPE $TMPM
> -}
> -
> -for flag in $gcc_flags; do
> -    if cc_has_warning_flag $flag ; then
> -        QEMU_CFLAGS="$QEMU_CFLAGS $flag"
> -    fi
> -    if objcc_has_warning_flag $flag ; then
> -        QEMU_OBJCFLAGS="$QEMU_OBJCFLAGS $flag"
> -    fi
> -done
> -
> -# Disable -Wmissing-braces on older compilers that warn even for
> -# the "universal" C zero initializer {0}.
> -cat > $TMPC << EOF
> -struct {
> -  int a[2];
> -} x = {0};
> -EOF
> -if compile_object "-Werror" "" ; then
> -  :
> -else
> -  QEMU_CFLAGS="$QEMU_CFLAGS -Wno-missing-braces"
> -fi
> -
>  # Resolve default for --enable-plugins
>  if test "$static" = "yes" ; then
>    if test "$plugins" = "yes"; then
> @@ -1928,7 +1835,6 @@ echo "NINJA=$ninja" >> $config_host_mak
>  echo "PKG_CONFIG=${pkg_config_exe}" >> $config_host_mak
>  echo "CC=$cc" >> $config_host_mak
>  echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak
> -echo "QEMU_OBJCFLAGS=$QEMU_OBJCFLAGS" >> $config_host_mak
>  echo "EXESUF=$EXESUF" >> $config_host_mak
>
>  # use included Linux headers
> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> index 8a316cd76f2f..b2b9db9f51af 100644
> --- a/contrib/plugins/Makefile
> +++ b/contrib/plugins/Makefile
> @@ -27,8 +27,7 @@ SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  # The main QEMU uses Glib extensively so it's perfectly fine to use it
>  # in plugins (which many example do).
>  CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0)
> -CFLAGS += -fPIC -Wall $(filter -W%, $(QEMU_CFLAGS))
> -CFLAGS += $(if $(findstring no-psabi,$(QEMU_CFLAGS)),-Wpsabi)
> +CFLAGS += -fPIC -Wall
>  CFLAGS += $(if $(CONFIG_DEBUG_TCG), -ggdb -O0)
>  CFLAGS += -I$(SRC_PATH)/include/qemu
>
> diff --git a/meson.build b/meson.build
> index c5a8dce9e1d6..4ec8104b73a3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -190,16 +190,8 @@ endif
>  # Compiler flags #
>  ##################
>
> -qemu_common_flags = []
> +qemu_common_flags = config_host['QEMU_CFLAGS'].split()
>  qemu_cflags = []
> -foreach arg : config_host['QEMU_CFLAGS'].split()
> -  if arg.startswith('-W')
> -    qemu_cflags += arg
> -  else
> -    qemu_common_flags += arg
> -  endif
> -endforeach
> -qemu_objcflags = config_host['QEMU_OBJCFLAGS'].split()
>  qemu_ldflags = []
>
>  if get_option('gprof')
> @@ -380,20 +372,49 @@ endif
>  add_global_arguments(qemu_common_flags, native: false, language: 
> all_languages)
>  add_global_link_arguments(qemu_ldflags, native: false, language: 
> all_languages)
>
> +# Collect warnings that we want to enable
> +
> +warn_flags = [
> +  '-Wundef',
> +  '-Wwrite-strings',
> +  '-Wmissing-prototypes',
> +  '-Wstrict-prototypes',
> +  '-Wredundant-decls',
> +  '-Wold-style-declaration',
> +  '-Wold-style-definition',
> +  '-Wtype-limits',
> +  '-Wformat-security',
> +  '-Wformat-y2k',
> +  '-Winit-self',
> +  '-Wignored-qualifiers',
> +  '-Wempty-body',
> +  '-Wnested-externs',
> +  '-Wendif-labels',
> +  '-Wexpansion-to-defined',
> +  '-Wimplicit-fallthrough=2',
> +  '-Wno-initializer-overrides',
> +  '-Wno-missing-include-dirs',
> +  '-Wno-shift-negative-value',
> +  '-Wno-string-plus-int',
> +  '-Wno-typedef-redefinition',
> +  '-Wno-tautological-type-limit-compare',
> +  '-Wno-psabi',
> +  '-Wno-gnu-variable-sized-type-not-at-end',
> +]
> +
> +if not cc.compiles('''
> +  struct {
> +    int a[2];
> +  } x = {0};''', args: ['-Werror'])
> +  warn_flags += ['-Wno-missing-braces']
> +endif
> +
>  # Check that the C++ compiler exists and works with the C compiler.
>  link_language = 'c'
>  linker = cc
>  qemu_cxxflags = []
>  if 'cpp' in all_languages
> -  add_global_arguments(['-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', 
> '-D__STDC_FORMAT_MACROS'],
> -                       native: false, language: 'cpp')
> -  foreach k: qemu_cflags
> -    if k not in ['-Wstrict-prototypes', '-Wmissing-prototypes', 
> '-Wnested-externs',
> -                 '-Wold-style-declaration', '-Wold-style-definition', 
> '-Wredundant-decls']
> -      qemu_cxxflags += [k]
> -    endif
> -  endforeach
> -
> +  qemu_cxxflags = ['-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', 
> '-D__STDC_FORMAT_MACROS'] + qemu_cflags
>    if cxx.links(files('scripts/main.c'), args: qemu_cflags)
>      link_language = 'cpp'
>      linker = cxx
> @@ -413,9 +434,16 @@ if get_option('optimization') != '0' and targetos == 
> 'linux'
>    endif
>  endif
>
> -add_project_arguments(qemu_cflags, native: false, language: 'c')
> -add_project_arguments(qemu_cxxflags, native: false, language: 'cpp')
> -add_project_arguments(qemu_objcflags, native: false, language: 'objc')
> +add_project_arguments(qemu_cflags, native: false, language: 'objc')
> +add_project_arguments(cc.get_supported_arguments(warn_flags), native: false, 
> language: 'c')
> +if 'cpp' in all_languages
> +  add_project_arguments(qemu_cxxflags, native: false, language: 'cpp')
> +  add_project_arguments(cxx.get_supported_arguments(warn_flags), native: 
> false, language: 'cpp')
> +endif
> +if 'objc' in all_languages
> +  # Note sanitizer flags are not applied to Objective-C sources!
> +  add_project_arguments(objc.get_supported_arguments(warn_flags), native: 
> false, language: 'objc')
> +endif
>  if targetos == 'linux'
>    add_project_arguments('-isystem', meson.current_source_dir() / 
> 'linux-headers',
>                          '-isystem', 'linux-headers',
> @@ -3927,7 +3955,7 @@ if 'cpp' in all_languages
>    summary_info += {'QEMU_CXXFLAGS':     ' '.join(qemu_common_flags + 
> qemu_cxxflags)}
>  endif
>  if 'objc' in all_languages
> -  summary_info += {'QEMU_OBJCFLAGS':    ' '.join(qemu_common_flags + 
> qemu_objcflags)}
> +  summary_info += {'QEMU_OBJCFLAGS':    ' '.join(qemu_common_flags)}
>  endif
>  summary_info += {'QEMU_LDFLAGS':      ' '.join(qemu_ldflags)}
>  summary_info += {'profiler':          get_option('profiler')}
> --
> 2.38.1
>
>


-- 
Marc-André Lureau