[PATCH v2] Remove the CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE switch

[PATCH v2] Remove the CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE switch

[Thomas Huth]

GCC supports "#pragma GCC diagnostic" since version 4.6, and
Clang seems to support it, too, since its early versions 3.x.
That means that our minimum required compiler versions all support
this pragma already and we can remove the test from configure and
all the related #ifdefs in the code.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Keep the !defined(__clang__) in coroutine-ucontext.c

 configure                 | 29 -----------------------------
 include/ui/gtk.h          |  4 ----
 include/ui/qemu-pixman.h  |  4 ----
 scripts/decodetree.py     | 12 ++++--------
 ui/gtk.c                  |  4 ----
 util/coroutine-ucontext.c |  4 ++--
 6 files changed, 6 insertions(+), 51 deletions(-)

diff --git a/configure b/configure
index ee6c3c6792..fbf119bbc0 100755
--- a/configure
+++ b/configure
@@ -5703,31 +5703,6 @@ if compile_prog "" "" ; then
     linux_magic_h=yes
 fi
 
-########################################
-# check whether we can disable warning option with a pragma (this is needed
-# to silence warnings in the headers of some versions of external libraries).
-# This test has to be compiled with -Werror as otherwise an unknown pragma is
-# only a warning.
-#
-# If we can't selectively disable warning in the code, disable -Werror so that
-# the build doesn't fail anyway.
-
-pragma_disable_unused_but_set=no
-cat > $TMPC << EOF
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wstrict-prototypes"
-#pragma GCC diagnostic pop
-
-int main(void) {
-    return 0;
-}
-EOF
-if compile_prog "-Werror" "" ; then
-    pragma_diagnostic_available=yes
-else
-    werror=no
-fi
-
 ########################################
 # check if we have valgrind/valgrind.h
 
@@ -7661,10 +7636,6 @@ if test "$linux_magic_h" = "yes" ; then
   echo "CONFIG_LINUX_MAGIC_H=y" >> $config_host_mak
 fi
 
-if test "$pragma_diagnostic_available" = "yes" ; then
-  echo "CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE=y" >> $config_host_mak
-fi
-
 if test "$valgrind_h" = "yes" ; then
   echo "CONFIG_VALGRIND_H=y" >> $config_host_mak
 fi
diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index d1b230848a..eaeb450f91 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -1,15 +1,11 @@
 #ifndef UI_GTK_H
 #define UI_GTK_H
 
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 /* Work around an -Wstrict-prototypes warning in GTK headers */
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wstrict-prototypes"
-#endif
 #include <gtk/gtk.h>
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 #pragma GCC diagnostic pop
-#endif
 
 #include <gdk/gdkkeysyms.h>
 
diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index 3b7cf70157..87737a6f16 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -7,14 +7,10 @@
 #define QEMU_PIXMAN_H
 
 /* pixman-0.16.0 headers have a redundant declaration */
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wredundant-decls"
-#endif
 #include <pixman.h>
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 #pragma GCC diagnostic pop
-#endif
 
 /*
  * pixman image formats are defined to be native endian,
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 530d41ca62..694757b6c2 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1327,12 +1327,10 @@ def main():
     # but we can't tell which ones.  Prevent issues from the compiler by
     # suppressing redundant declaration warnings.
     if anyextern:
-        output("#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE\n",
-               "# pragma GCC diagnostic push\n",
-               "# pragma GCC diagnostic ignored \"-Wredundant-decls\"\n",
-               "# ifdef __clang__\n"
+        output("#pragma GCC diagnostic push\n",
+               "#pragma GCC diagnostic ignored \"-Wredundant-decls\"\n",
+               "#ifdef __clang__\n"
                "#  pragma GCC diagnostic ignored \"-Wtypedef-redefinition\"\n",
-               "# endif\n",
                "#endif\n\n")
 
     out_pats = {}
@@ -1347,9 +1345,7 @@ def main():
     output('\n')
 
     if anyextern:
-        output("#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE\n",
-               "# pragma GCC diagnostic pop\n",
-               "#endif\n\n")
+        output("#pragma GCC diagnostic pop\n\n")
 
     for n in sorted(formats.keys()):
         f = formats[n]
diff --git a/ui/gtk.c b/ui/gtk.c
index d4b49bd7da..b0cc08ad6d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1996,14 +1996,10 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
              * proper replacement (native opengl support) is only
              * available in 3.16+.  Silence the warning if possible.
              */
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
             gtk_widget_set_double_buffered(vc->gfx.drawing_area, FALSE);
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 #pragma GCC diagnostic pop
-#endif
             vc->gfx.dcl.ops = &dcl_egl_ops;
         }
     } else
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index f0b66320e1..36594d381f 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -237,8 +237,8 @@ Coroutine *qemu_coroutine_new(void)
 }
 
 #ifdef CONFIG_VALGRIND_H
-#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && !defined(__clang__)
 /* Work around an unused variable in the valgrind.h macro... */
+#if !defined(__clang__)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #endif
@@ -246,7 +246,7 @@ static inline void valgrind_stack_deregister(CoroutineUContext *co)
 {
     VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
 }
-#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && !defined(__clang__)
+#if !defined(__clang__)
 #pragma GCC diagnostic pop
 #endif
 #endif
-- 
2.18.1

Re: [PATCH v2] Remove the CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE switch

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

On Fri, Jul 10, 2020 at 06:55:15AM +0200, Thomas Huth wrote:
> GCC supports "#pragma GCC diagnostic" since version 4.6, and
> Clang seems to support it, too, since its early versions 3.x.
> That means that our minimum required compiler versions all support
> this pragma already and we can remove the test from configure and
> all the related #ifdefs in the code.
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Keep the !defined(__clang__) in coroutine-ucontext.c
> 
>  configure                 | 29 -----------------------------
>  include/ui/gtk.h          |  4 ----
>  include/ui/qemu-pixman.h  |  4 ----
>  scripts/decodetree.py     | 12 ++++--------
>  ui/gtk.c                  |  4 ----
>  util/coroutine-ucontext.c |  4 ++--
>  6 files changed, 6 insertions(+), 51 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] Remove the CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE switch

[Peter Maydell]

On Fri, 10 Jul 2020 at 05:56, Thomas Huth <thuth@redhat.com> wrote:
>
> GCC supports "#pragma GCC diagnostic" since version 4.6, and
> Clang seems to support it, too, since its early versions 3.x.
> That means that our minimum required compiler versions all support
> this pragma already and we can remove the test from configure and
> all the related #ifdefs in the code.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Keep the !defined(__clang__) in coroutine-ucontext.c

If we're going to mandate "at least gcc 4.6 or clang", perhaps
we should have a sanity check for it, something like

#if !defined __clang__
# if !QEMU_GNUC_PREREQ(4, 6)
#  error QEMU requires at least GCC 4.6
# endif
#endif

(maybe also check clang version, though that is more awkward
because upstream clang and Apple's compiler set the version
number defines differently.)

We could put that in compiler.h. Checking in configure would be
more userfriendly but maybe a little more effort.
The other advantage of this check is we have effectively
some internal documentation of our current minimum
compiler requirement.

There is also some tidying up that can then be done:
several places in the code use QEMU_GNU_PREREQ() to insist
on "at least gcc 4.4" or "at least gcc 4.6", and that
ifdeffery can probably be removed (though some caution
is required as I think even modern clang may advertise
itself as gcc 4.2. In some cases __has_whatever feature
tests may be usable instead.)

thanks
-- PMM

Re: [PATCH v2] Remove the CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE switch

[Thomas Huth]

On 10/07/2020 15.25, Peter Maydell wrote:
> On Fri, 10 Jul 2020 at 05:56, Thomas Huth <thuth@redhat.com> wrote:
>>
>> GCC supports "#pragma GCC diagnostic" since version 4.6, and
>> Clang seems to support it, too, since its early versions 3.x.
>> That means that our minimum required compiler versions all support
>> this pragma already and we can remove the test from configure and
>> all the related #ifdefs in the code.
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  v2: Keep the !defined(__clang__) in coroutine-ucontext.c
> 
> If we're going to mandate "at least gcc 4.6 or clang", perhaps
> we should have a sanity check for it, something like
> 
> #if !defined __clang__
> # if !QEMU_GNUC_PREREQ(4, 6)
> #  error QEMU requires at least GCC 4.6
> # endif
> #endif
> 
> (maybe also check clang version, though that is more awkward
> because upstream clang and Apple's compiler set the version
> number defines differently.)
> 
> We could put that in compiler.h. Checking in configure would be
> more userfriendly but maybe a little more effort.
> The other advantage of this check is we have effectively
> some internal documentation of our current minimum
> compiler requirement.

It's already there. Look for:

 "You need at least GCC v4.8 or Clang v3.4 (or XCode Clang v5.1)"

in the configure script.

> There is also some tidying up that can then be done:
> several places in the code use QEMU_GNU_PREREQ() to insist
> on "at least gcc 4.4" or "at least gcc 4.6",
Oh, weird, I thought we got rid of all of them when we bumped the
minimum requirement to GCC 4.8 ... these spots could certainly be
cleaned up nowadays.

 Thomas

Re: [PATCH v2] Remove the CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE switch

[Peter Maydell]

On Fri, 10 Jul 2020 at 15:09, Thomas Huth <thuth@redhat.com> wrote:
>
> On 10/07/2020 15.25, Peter Maydell wrote:
> > If we're going to mandate "at least gcc 4.6 or clang", perhaps
> > we should have a sanity check for it

> It's already there. Look for:
>
>  "You need at least GCC v4.8 or Clang v3.4 (or XCode Clang v5.1)"
>
> in the configure script.

Whoops, should have checked there :-)

thanks
-- PMM