[Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Pranith Kumar]

Clang produces the following warning. The warning is detailed here:
https://reviews.llvm.org/D15866. Disable the warning.

/home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if SPICE_NEEDS_SET_MM_TIME
    ^
/home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
    ^
/home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if SPICE_NEEDS_SET_MM_TIME
    ^
/home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 configure | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/configure b/configure
index f57fcc6..a1a6e3f 100755
--- a/configure
+++ b/configure
@@ -3025,6 +3025,19 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
     fi
 fi
 
+# Silence clang warnings about expansion to 'defined' in macro
+cat > $TMPC << EOF
+#define TEST (defined(DUMMY))
+#if TEST
+int main(void) { return 0; }
+#endif
+EOF
+if ! compile_prog "-Werror" ; then
+    if cc_has_warning_flag "-Wno-expansion-to-defined"; then
+	CFLAGS="-Wno-expansion-to-defined $CFLAGS"
+    fi
+fi
+
 ##########################################
 # SHA command probe for modules
 if test "$modules" = yes; then
-- 
2.9.2

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Paolo Bonzini]

On 09/08/2016 04:35, Pranith Kumar wrote:
> Clang produces the following warning. The warning is detailed here:
> https://reviews.llvm.org/D15866. Disable the warning.
> 
> /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
>     ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
>     ^
> /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
>     ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> 
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> ---
>  configure | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/configure b/configure
> index f57fcc6..a1a6e3f 100755
> --- a/configure
> +++ b/configure
> @@ -3025,6 +3025,19 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
>      fi
>  fi
>  
> +# Silence clang warnings about expansion to 'defined' in macro
> +cat > $TMPC << EOF
> +#define TEST (defined(DUMMY))
> +#if TEST
> +int main(void) { return 0; }
> +#endif
> +EOF
> +if ! compile_prog "-Werror" ; then
> +    if cc_has_warning_flag "-Wno-expansion-to-defined"; then
> +	CFLAGS="-Wno-expansion-to-defined $CFLAGS"
> +    fi
> +fi
> +
>  ##########################################
>  # SHA command probe for modules
>  if test "$modules" = yes; then
> 

Can you just add it to gcc_flags instead?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Peter Maydell]

On 9 August 2016 at 03:35, Pranith Kumar <bobby.prani@gmail.com> wrote:
> Clang produces the following warning. The warning is detailed here:
> https://reviews.llvm.org/D15866. Disable the warning.
>
> /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
>     ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
>     ^
> /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
>     ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))

Is this the only thing in our code which provokes the warning?
If so, why don't we just fix it to not be undefined behaviour?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Daniel P. Berrange]

On Tue, Aug 09, 2016 at 09:09:13AM +0100, Peter Maydell wrote:
> On 9 August 2016 at 03:35, Pranith Kumar <bobby.prani@gmail.com> wrote:
> > Clang produces the following warning. The warning is detailed here:
> > https://reviews.llvm.org/D15866. Disable the warning.
> >
> > /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> > #if SPICE_NEEDS_SET_MM_TIME
> >     ^
> > /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
> >   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> >     ^
> > /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> > #if SPICE_NEEDS_SET_MM_TIME
> >     ^
> > /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
> >   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> 
> Is this the only thing in our code which provokes the warning?
> If so, why don't we just fix it to not be undefined behaviour?

Indeed, it looks like it could be fixed by changing include/ui/qemu-spice.h


#define SPICE_NEEDS_SET_MM_TIME                                       \
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))


to be

#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
#define SPICE_NEEDS_SET_MM_TIME
#endif


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Paolo Bonzini]

On 09/08/2016 10:09, Peter Maydell wrote:
> On 9 August 2016 at 03:35, Pranith Kumar <bobby.prani@gmail.com> wrote:
>> Clang produces the following warning. The warning is detailed here:
>> https://reviews.llvm.org/D15866. Disable the warning.
>>
>> /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
>> #if SPICE_NEEDS_SET_MM_TIME
>>     ^
>> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
>>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
>>     ^
>> /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
>> #if SPICE_NEEDS_SET_MM_TIME
>>     ^
>> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
>>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> 
> Is this the only thing in our code which provokes the warning?
> If so, why don't we just fix it to not be undefined behaviour?

Indeed that was my first suggestion yesterday.  However, we use -Wundef
(a good idea, I think), and fixing it would cause the -Wundef warning to
appear instead.

Paolo

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Peter Maydell]

On 9 August 2016 at 09:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 09/08/2016 10:09, Peter Maydell wrote:
>> Is this the only thing in our code which provokes the warning?
>> If so, why don't we just fix it to not be undefined behaviour?
>
> Indeed that was my first suggestion yesterday.  However, we use -Wundef
> (a good idea, I think), and fixing it would cause the -Wundef warning to
> appear instead.

Why would
#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
#define SPICE_NEEDS_SET_MM_TIME 1
#else
#define SPICE_NEEDS_SET_MM_TIME 0
#endif

provoke a Wundef warning?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Paolo Bonzini]

On 09/08/2016 10:17, Peter Maydell wrote:
> On 9 August 2016 at 09:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 09/08/2016 10:09, Peter Maydell wrote:
>>> Is this the only thing in our code which provokes the warning?
>>> If so, why don't we just fix it to not be undefined behaviour?
>>
>> Indeed that was my first suggestion yesterday.  However, we use -Wundef
>> (a good idea, I think), and fixing it would cause the -Wundef warning to
>> appear instead.
> 
> Why would
> #if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
> #define SPICE_NEEDS_SET_MM_TIME 1
> #else
> #define SPICE_NEEDS_SET_MM_TIME 0
> #endif
> 
> provoke a Wundef warning?

Oh, that would work indeed.  I was thinking of simplifying the
definition of SPICE_NEEDS_SET_MM_TIME like this:

#define SPICE_NEEDS_SET_MM_TIME (SPICE_SERVER_VERSION < 0xc06)

If we do this, it would make sense to add -Wexpansion-to-defined to GCC
and enable it (though I still believe it's wrong to add it to -Wall).

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

[Pranith Kumar]

On Tue, Aug 9, 2016 at 4:17 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> #if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
> #define SPICE_NEEDS_SET_MM_TIME 1
> #else
> #define SPICE_NEEDS_SET_MM_TIME 0
> #endif
>

This looks better. I will fix up a patch.

-- 
Pranith