Bug 104948 - When '&&' has non bool parameters, a better warning should be generated
Summary: When '&&' has non bool parameters, a better warning should be generated
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 12.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-16 08:42 UTC by dagelf
Modified: 2022-03-17 10:43 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dagelf 2022-03-16 08:42:51 UTC
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.
Comment 1 dagelf 2022-03-16 08:44:17 UTC
To boot, most other C compilers interpret `&&` as a logical AND.
Comment 2 Andrew Pinski 2022-03-16 08:51:03 UTC
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;
Comment 3 dagelf 2022-03-16 09:07:00 UTC
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.
Comment 4 Andrew Pinski 2022-03-16 09:10:07 UTC
(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;
}
Comment 5 Andrew Pinski 2022-03-16 09:10:55 UTC
(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.
Comment 6 Richard Biener 2022-03-16 09:13:09 UTC
(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;
}
Comment 7 Jonathan Wakely 2022-03-16 09:33:18 UTC
Until you clear up your confusion, please stop leaving completely incorrect comments all over bugzilla.
Comment 8 dagelf 2022-03-16 11:20:06 UTC
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.
Comment 9 Jonathan Wakely 2022-03-16 12:34:57 UTC
(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.
Comment 10 Jakub Jelinek 2022-03-16 12:40:45 UTC
(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.
Comment 11 Andreas Schwab 2022-03-16 12:56:30 UTC
The size of bool is target dependent (though only Darwin/ppc overrides it).
Comment 12 Jonathan Wakely 2022-03-16 13:04:25 UTC
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
Comment 13 dagelf 2022-03-17 09:03:06 UTC
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.
Comment 14 Jonathan Wakely 2022-03-17 10:43:18 UTC
(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.