[Bug c/104948] New: When '&&' present in a comparison, a warning should be generated

[Bug c/104948] New: When '&&' present in a comparison, a warning should be generated

[coenraad at wish dot org.za]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

            Bug ID: 104948
           Summary: When '&&' present in a comparison, a warning should be
                    generated
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: coenraad at wish dot org.za
  Target Milestone: ---

The sheer number of issues reported here that wrongly use '&&' as a logical
AND, warrants that this warning is direly necessary.

Even worse, the presence of '&&' in an evaluation, causes that branch to be
optimized out, completely, no matter what optimization level is used.

The reason for so many people using '&&' as logical AND, is because that is
what it is in almost every other language in widespread use.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[coenraad at wish dot org.za]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #1 from dagelf <coenraad at wish dot org.za> ---
To boot, most other C compilers interpret `&&` as a logical AND.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
unary operator vs binary operator.

You are just confused really.

that is if used as an unary operator, it will taken as the address of the
lable. While used as a binary operator it will be used as you expect.

Also the warning already happens with -Waddress (which is enabled with -Wall)"
<source>: In function 'f':
<source>:3:9: warning: the address of 'a' will always evaluate as 'true'
[-Waddress]
     if (&&a) return 1;

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[coenraad at wish dot org.za]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #3 from dagelf <coenraad at wish dot org.za> ---
Can you please give me an example of code with the generated machine code where
it actually works as a logical AND? 

Because I've tried several versions, and under no circumstances did it function
as a binary operator. 

The warning might be improved to state that `&&` is not logical AND.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to dagelf from comment #3)
> Can you please give me an example of code with the generated machine code
> where it actually works as a logical AND? 
> 
> Because I've tried several versions, and under no circumstances did it
> function as a binary operator. 
> 
> The warning might be improved to state that `&&` is not logical AND.

int f(int *a, int *b)
{
    if (*a && *b)
      return 1;
    return 0;
}

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to dagelf from comment #3)
> > Can you please give me an example of code with the generated machine code
> > where it actually works as a logical AND? 
> > 
> > Because I've tried several versions, and under no circumstances did it
> > function as a binary operator. 
> > 
> > The warning might be improved to state that `&&` is not logical AND.
> 
> int f(int *a, int *b)
> {
>     if (*a && *b)
>       return 1;
>     return 0;
> }

Note in C, && is short cutting that means *a != 0 has to be true before
evaluating *b != 0.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to dagelf from comment #3)
> Can you please give me an example of code with the generated machine code
> where it actually works as a logical AND? 
> 
> Because I've tried several versions, and under no circumstances did it
> function as a binary operator. 
> 
> The warning might be improved to state that `&&` is not logical AND.

Maybe you can provide one of the several versions where it does not work to
enlight us.  Even the following works

int f(int a)
{
L:
  if (a && && L)
    return 1;
  return 0;
}

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Until you clear up your confusion, please stop leaving completely incorrect
comments all over bugzilla.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[coenraad at wish dot org.za]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #8 from dagelf <coenraad at wish dot org.za> ---
Makes perfect sense now. && is "logical" in that it can only produce a bool,
which in C is an int and anything except 0 or 1 is evaluated to false at
compile time. 

There was a time when 'logical' and 'bitwise' were used interchangeably, based
on the fact that 'boolean operators' work on 'boolean logic'. 

This is what lead me here:

$ cat test.c
int f(int a) {
  if ((a && 12) == 12 ) 
     return 11;
  return 10;
}

$ gcc -c -O0 test.c && objdump -d test1.o
test1.o:     file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <f>:
   0:   55                      push   %rbp
   1:   48 89 e5                mov    %rsp,%rbp
   4:   89 7d fc                mov    %edi,-0x4(%rbp)
   7:   b8 00 00 00 00          mov    $0xa,%eax
   c:   5d                      pop    %rbp
   d:   c3                      retq   

With a single `&` it works as expected. 

In my defence, when I last did a C course all boolean operators were bitwise. I
suddenly feel really old that even C has changed. Even the definition of
'logical' and 'bitwise' has changed. 

Apologies for not testing the obvious '-Wall'. 

Also apologies for just skimming over the output of icc, clang and msvc... I
just noticed that they include jumps where gcc didn't, so I was mistaken. 

The optimizations are impressive.

Still, searching for the issues logged here with '&&' in an evaluation, does
point to the fact that the error message could be improved. Might I recommend
'non-bitwise boolean' in the message instead of just 'boolean'. Or even better,
add '(did you mean bitwise AND & instead of &&) if that's present.

$ gcc -Wall -c -O0  test.c 
test1.c: In function ‘f’:
test1.c:5:22: warning: comparison of constant ‘12’ with boolean expression is
always false (Did you mean & instead of &&?) [-Wbool-compare] 

Compare to "warning: comparison of constant ‘12’ with non-bitwise boolean
expression is always false [-Wbool-compare]" might lead to less confusion.

When expecting the result of an '&&' evaluation to be a bitwise AND, this
distinction can make a world of difference and could've pointed at least me in
the right direction.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to dagelf from comment #8)
> Makes perfect sense now. && is "logical" in that it can only produce a bool,
> which in C is an int and anything except 0 or 1 is evaluated to false at
> compile time. 

No, in C bool is a distinct data type, and sizeof(bool) == 1.

Values of that type other than 0 or 1 result in undefined behaviour.


> 
> There was a time when 'logical' and 'bitwise' were used interchangeably,
> based on the fact that 'boolean operators' work on 'boolean logic'. 
> 
> This is what lead me here:
> 
> $ cat test.c
> int f(int a) {
>   if ((a && 12) == 12 ) 

This will never be true.

The result of (a && 12) is either 0 or 1, and so never equal to 12.


>      return 11;
>   return 10;
> }
> 
> $ gcc -c -O0 test.c && objdump -d test1.o
> test1.o:     file format elf64-x86-64
> Disassembly of section .text:
> 0000000000000000 <f>:
>    0:	55                   	push   %rbp
>    1:	48 89 e5             	mov    %rsp,%rbp
>    4:	89 7d fc             	mov    %edi,-0x4(%rbp)
>    7:	b8 00 00 00 00       	mov    $0xa,%eax
>    c:	5d                   	pop    %rbp
>    d:	c3                   	retq   
> 
> With a single `&` it works as expected. 

Your expectation is wrong.

> 
> In my defence, when I last did a C course all boolean operators were
> bitwise.

I doubt that is true.


> I suddenly feel really old that even C has changed. Even the
> definition of 'logical' and 'bitwise' has changed. 

I don't think that's true either.


> Compare to "warning: comparison of constant ‘12’ with non-bitwise boolean
> expression is always false [-Wbool-compare]" might lead to less confusion.

It would confuse people who know C, because "non-bitwise boolean expression" is
meaningless.

> When expecting the result of an '&&' evaluation to be a bitwise AND,

Your expectation is simply wrong, that's not how C works. We can't write
diagnostics to suit every potential misunderstanding of how C works.

The warning text is accurate and correct.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> (In reply to dagelf from comment #8)
> > Makes perfect sense now. && is "logical" in that it can only produce a bool,
> > which in C is an int and anything except 0 or 1 is evaluated to false at
> > compile time. 
> 
> No, in C bool is a distinct data type, and sizeof(bool) == 1.

_Bool, that is.  bool is when stdbool.h is included a define to _Bool.
Though, && result is int in C, not _Bool, while in C++ bool.

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[schwab@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #11 from Andreas Schwab <schwab@linux-m68k.org> ---
The size of bool is target dependent (though only Darwin/ppc overrides it).

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, to be more precise:

&& produces an int (not a _Bool / bool) with value 0 or 1.

_Bool (a.k.a bool) is a distinct type, which might be larger or smaller than
int (the only actual requirement is "large enough to store the values 0 and
1").

The name 'bool' is currently just a macro for _Bool defined in <stdbool.h> but
C23 might change it to a proper keyword:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2934.pdf

[Bug c/104948] When '&&' present in a comparison, a warning should be generated

[coenraad at wish dot org.za]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #13 from dagelf <coenraad at wish dot org.za> ---
Yes, I seem to have had a hole in my head. Sorry. For reference, to summarise
then:

& returns only the bits that are the same (bitwise AND) (or the bits that are
left after AND) (which will evaluate to true unless its 0 eg. (2 & 1) is false
but (3 & 1) is true. 

&& casts operands to bool, so anything nonzero becomes 1. (eg. (2 && 1) is
true)

&& and & nonequivalence: https://godbolt.org/z/Yf5cxcKch

So & and && are only interchangeable when operating on bool parameters, on
anything else they each do something completely different. 

The fact that there's a warning is great. Perhaps its fine. But this is
something so fundamental, it feels like no room for confusion should be left.
In fact, the more I think about it, the more I am convinced that it should not
be a warning, but an error, so -Werror is my new default. 

Although I would love to find counter examples, I would be willing to wager
that && mixed with non bools, will almost always be done in error.

[Bug c/104948] When '&&' has non bool parameters, a better warning should be generated

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104948

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to dagelf from comment #13)
> Although I would love to find counter examples, I would be willing to wager
> that && mixed with non bools, will almost always be done in error.

No, this is valid and not done in error:

void f(const char* str, size_t len)
{
  if (str && len)
  { /* ... */ }
}

The condition is equivalent to (str != 0 && len != 0) but more succinct. If you
prefer the more verbose form, that's fine, but it doesn't mean everyone else's
code is erroneous.