bugprone-assignment-in-if-condition false positive on GNU statement expression
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
To reproduce:
```c++
#define MACRO(arg) \
({ \
bool __r; \
__r = (arg) != 1; \
__r; \
})
int main(int argc, [[maybe_unused]] char* argv[]) {
if (MACRO(argc))
return 42;
return 56;
}
```
(paths and messages about compilation database elided)
```
$ clang-tidy --checks=bugprone-assignment-in-if-condition bug.cpp
bug.cpp:9:9: error: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition,-warnings-as-errors]
9 | if (MACRO(argc))
| ^
bug.cpp:4:13: note: expanded from macro 'MACRO'
4 | __r = (arg) != 1; \
| ~~~~^~~~~~~~~~~~
bug.cpp:9:9: note: if it should be an assignment, move it out of the 'if' condition
9 | if (MACRO(argc))
| ^
bug.cpp:4:13: note: expanded from macro 'MACRO'
4 | __r = (arg) != 1; \
| ^
bug.cpp:9:9: note: if it is meant to be an equality check, change '=' to '=='
9 | if (MACRO(argc))
| ^
bug.cpp:4:13: note: expanded from macro 'MACRO'
4 | __r = (arg) != 1; \
| ^
```
This is debatably not a false positive, as there is literally an assignment inside the `if` condition, but usage inside a GNU statement-expression extension doesn't seem like the sort of usage the check is meant to guard against?
This comes up in real code because several macros in the GLib library take advantage of statement-expressions on platforms that support them to provide a more performant definition of the macro than the fallback definition on platforms without.
If it's not desired to allow this usage in the check, perhaps the check could gain an `IgnoreMacros` config option like several other checks have? That would solve my problem in another way, at least.
Contributor guide
Research direction
Start with the provided C++ reproducer and the clang-tidy bugprone-assignment-in-if-condition check. Determine whether GNU statement expressions should be exempted or whether an IgnoreMacros configuration option is the intended fix, then add coverage for the chosen behavior and verify the check no longer reports the reported false positive.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- devtools, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100