Re: [Bug 1881004] [NEW] fpu/softfloat.c: error: bitwise negation of a boolean expression

[Thomas Huth <thuth@redhat.com>]

On 27/05/2020 23.54, Eric Blake wrote:
> On 5/27/20 4:40 PM, Peter Maydell wrote:
>> On Wed, 27 May 2020 at 20:21, Philippe Mathieu-Daudé
>> <1881004@bugs.launchpad.net> wrote:
>>>
>>> Public bug reported:
>>>
>>> Last time I built QEMU was on commit
>>> d5c75ec500d96f1d93447f990cd5a4ef5ba27fae,
>>> I just pulled to fea8f3ed739536fca027cf56af7f5576f37ef9cd and now get:
>>>
>>>    CC      lm32-softmmu/fpu/softfloat.o
>>> fpu/softfloat.c:3365:13: error: bitwise negation of a boolean
>>> expression; did you mean logical negation? [-Werror,-Wbool-operation]
>>>      absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
>>>              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>              !
>>
>>
>> "(x & y)" is not a boolean expression, so we should report this to clang
>> as a bug (I assume what they actually are trying to complain about is
>> bitwise AND with a boolean expression).
> 
> We have:
> 
> uint64_t &= ~ ( ( ( int8_t ^ int ) == int ) & bool )
> 
> which is
> 
> uint64_t &= ~ ( bool & bool )
> 
> which is then
> 
> uint64_t &= ~ ( int )
> 
> resulting in one of:
> 
> uint64_t &= 0xffffffffffffffff
> uint64_t &= 0xfffffffffffffffe
> 
> It is a very odd way of stating that 'if this condition is true, mask
> out the least-significant-bit'.  In general, 'bool & bool' is used where
> the side-effect-skipping 'bool && bool' is inappropriate; I'm a bit
> surprised that clang is not questioning whether we meant '&&' instead of
> '&' (the two operators give the same effect in this case).
> 
> You are right that clang is fishy for calling it logical negation of a
> bool, when it is really logical negation of an int, but we are also
> fishy in that we are using bitwise AND of two bools as an int in the
> first place.
> 
> Regardless of whether clang changes, would it be better to write the
> code as:
> 
> if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
>     absZ &= ~1;
> }

I agree, that's also much better to read.
And FWIW, WinUAE fixed a similar problem in the same way recently:

 https://github.com/tonioni/WinUAE/commit/51f5e7bfc39cf37daf7283

So I think this is the right way to go. Could you send your suggestion
as a proper patch?

 Thanks,
  Thomas