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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
