alexfh requested changes to this revision. This revision now requires changes to proceed.
================ Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:30 @@ +29,3 @@ + + Finder->addMatcher(implicitCastExpr(hasIntegerLiteralCastToBool, + unless(hasParent(explicitCastExpr()))), ---------------- Adding two matchers that do almost the same checks is wasteful. You can do the same in a single matcher, which will do twice less work: implicitCastExpr(has(integerLiteral().bind("literal")), hasImplicitDestinationType(qualType(booleanType())), unless(isInTemplateInstantiation()), hasParent(anyOf(explicitCastExpr().bind("cast"), anything()))) (if `anything()` here doesn't work, you can probably use `expr()` or something else). ================ Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:56 @@ +55,3 @@ + "converting integer literal to " + "bool%select{| inside a macro}0, use bool literal instead"); + ---------------- Can you explain, why is it important to note that this happens "inside a macro"? ================ Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:58 @@ +57,3 @@ + + if (isPreprocessorDependent(Literal) || isPreprocessorDependent(Cast)) { + Diag << 1; ---------------- I think, you can write `if (Cast) Literal = Cast;` and the rest of the code will be much shorter. Also, the `isPreprocessorDependent` doesn't seem to be needed. http://reviews.llvm.org/D18745 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits