[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