- Add readability-simplify-boolean-expr test cases for member variables
Thanks for the patch. Is this revision dependent on D56303 (or other way around)?
The const for these locals in uncommon in clang-tidy, given its a value type. I am not strongly against them, but would prefer consistency.
I didn't see a test that utilized this struct, did I overlook it?
Implicit conversion to bool-cases would be interesting as well. Maybe implicit conversion to integral in general.
Would bitfields with a single bit be of interest as well?
For this case the logical inversion is simple an obviously <=, but what if the condition is more complex?
I would personally prefer !(i < 20) instead + complex logical expressions as additional test. They would be interesting for the if cases, too.
I can undo the const.
Leftover from copy/paste of readability-simplify-bool-expr.cpp, I'll remove it.
Implicit conversion cases are covered by the other file. Here, I'm just testing the cases that interact with member variables because the matcher needs to use memberExpr() and not just declRefExpr()
That could be an enhancement, but it's not addressed by this patch.
If you have complex conditions, you can run the check repeatedly until you reach a fixed point.
Using !(x) instead of the reverse operator could be added as an enhancement, but it is not covered by this patch.
Ok, that would be great.
Which other file? The other revision?
But the current version is the enhancement.
I based this new file off readability-simplify-bool-expr.cpp that is already there. It has all the main test cases for this check.
No, the current version is a bug fix. The simplification of boolean expressions was already there, but it didn't handle member variables properly.
You're asking for an additional enhancement about the way boolean expressions are simplified. That's fine for an additional enhancement of the check, but it is not the point of this patch.
The complex expression you're discussing in your comment is not changed by this change, nor is it changed by the existing check.
Did you check if the changes actually fix the problematic behaviour? Did you run it over any project and did the code-transformation break the code, false positives, ...?
The test cases that I added covered the behavior described in the bug report. I didn't run it over clang to see if it introduced any false positives, I can do that. However, when I originally wrote this check it was pretty well tested and I have confidence that I haven't regressed. However, it's still worth testing, so I will do that.
Uh... I'm developing on Windows, which doesn't have a solution for generating compile_commands.json, so I can't easily test this on the entire clang codebase. I will test on a source file based on the original bug report.
I managed to do some limited testing on the original source file from the bug report in lldb and verified that the correct fix was applied there. I also tried a few other files and verified no false positives.