llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.96k stars 11.54k forks source link

[Clang] `-Wparentheses-equality` produces a false positive for `==` fold expressions in conditions #101863

Open Sirraide opened 1 month ago

Sirraide commented 1 month ago

The following code (https://godbolt.org/z/rPqP4r55b):

void foo(auto... args) {
    if (((args == 0) or ...)) {}
}

void f() { 
    foo(3);
}

results in a -Wparentheses-equality warning:

<source>:2:16: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
    2 |     if (((args == 0) or ...)) {}
      |           ~~~~~^~~~
<source>:6:5: note: in instantiation of function template specialization 'foo<int>' requested here
    6 |     foo(3);
      |     ^
<source>:2:16: note: remove extraneous parentheses around the comparison to silence this warning
    2 |     if (((args == 0) or ...)) {}
      |          ~     ^   ~
<source>:2:16: note: use '=' to turn this equality comparison into an assignment
    2 |     if (((args == 0) or ...)) {}
      |                ^~
      |                =

However, the suggestion to remove parentheses here is obviously wrong: neither pair of parentheses can be removed, as that would result in an invalid fold expression (because args == 0 is not a cast-expression). It seems that, generally speaking, we should not issue this warning if the equality comparison is the operand of a fold expression.

llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-frontend

Author: None (Sirraide)

The following code (https://godbolt.org/z/rPqP4r55b): ```c++ void foo(auto... args) { if (((args == 0) or ...)) {} } void f() { foo(3); } ``` results in a `-Wparentheses-equality` warning: ``` <source>:2:16: warning: equality comparison with extraneous parentheses [-Wparentheses-equality] 2 | if (((args == 0) or ...)) {} | ~~~~~^~~~ <source>:6:5: note: in instantiation of function template specialization 'foo<int>' requested here 6 | foo(3); | ^ <source>:2:16: note: remove extraneous parentheses around the comparison to silence this warning 2 | if (((args == 0) or ...)) {} | ~ ^ ~ <source>:2:16: note: use '=' to turn this equality comparison into an assignment 2 | if (((args == 0) or ...)) {} | ^~ | = ``` However, the suggestion to remove parentheses here is obviously wrong: neither pair of parentheses can be removed, as that would result in an invalid fold expression (because `args == 0` is not a cast-expression). It seems that, generally speaking, we should not issue this warning if the equality comparison is the operand of a fold expression.
Sirraide commented 1 month ago

The culprit seems to be the fact that the AST we get after instantiating this is:

`-CompoundStmt 0x289d21d8 <col:24, line:3:1>
  `-IfStmt 0x289d21b8 <line:2:5, col:32>
    |-ParenExpr 0x289d2198 <col:10, col:20> 'bool'
    | `-BinaryOperator 0x289d2178 <col:11, col:19> 'bool' '=='
    |   |-ImplicitCastExpr 0x289d2160 <col:11> 'int' <LValueToRValue>
    |   | `-DeclRefExpr 0x289d2128 <col:11> 'int' lvalue ParmVar 0x289d1e18 'args' 'int'
    |   `-IntegerLiteral 0x289d1a98 <col:19> 'int' 0
    `-CompoundStmt 0x289d1b38 <col:31, col:32>

which looks exactly as though the user had written if ((args == 0)) {}.