[PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers
Richard via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 2 09:19:25 PST 2016
LegalizeAdulthood added inline comments.
================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:366
@@ +365,3 @@
+ binaryOperator(
+ isExpansionInMainFile(), hasOperatorName(OperatorName),
+ hasLHS(allOf(
----------------
aaron.ballman wrote:
> Sorry for not noticing this earlier, but since we have two other in-flight patches I reviewed this morning, it caught my attention: we should not be using isExpansionInMainFile, but instead testing the source location of the matches to see if they're in a macro expansion. This is usually done with something like `Result.SourceManager->isMacroBodyExpansion(SomeLoc)`.
>
> I realize this is existing code and not your problem, but it should be fixed in a follow-on patch (by someone) and include some macro test cases.
There is an open bug on this that I will address. And while this is existing code, I am the author of the original check :).
I also experienced a problem when running this check on some code that did something like:
```
#define FOO (par[1] > 4)
// ...
bool f() {
if (!FOO) {
return true;
} else {
return false;
}
}
```
So it needs improvement w.r.t. macros and that is also on my to-do list.
I'm trying to do things in incremental steps and not giant changes.
http://reviews.llvm.org/D16308
More information about the cfe-commits
mailing list