[clang-tidy] Add readability-implicit-bool-cast check

[piotrdz]

Summary

This is another check that I ported to clang-tidy from my colobot-lint tool.

As previously discussed on cfe-dev mailing list, this is one of those checks that I think is general and useful enough for contribution to clang-tidy.

This patch contains implementation of check taken from colobot-lint, but also extended a great deal, including FixIt hints for automated refactoring, exhaustive testcases, and user documentation.

[Eugene.Zelenko]

I agree with Piotr idea to introduce option to enable/disable check for pointer casts. Same should be done for integers where zero/non-zero value is used by system calls to indicate success/failure or in case of strings – end of string. Without such options check may be too verbose on legacy code bases.

May be dedicated category for casts checks should be created? We have google-readability-casting (through it duplicates in many aspects -Wold-style-cast, see PR24370), cppcoreguidelines-pro-type-const-cast and cppcoreguidelines-pro-type-reinterpret-cast. Other possible check is PR24525.

[sbenza]

Thanks for the check.
I know a lot of people that would appreciate if these implicit conversions didn't exist. =)

clang-tidy/readability/ImplicitBoolCastCheck.cpp

32 ↗

(On Diff #37054)

We should move most of these matchers to ASTMatchers.h as they are very general.
For example, we already have isInteger() as a QualType matcher so adding isBool() makes sense.

36 ↗

(On Diff #37054)

Maybe name it isMacroExpansion like the functions in SourceManager.

39 ↗

(On Diff #37054)

Please use the punctuation symbols for the boolean operators. (ie || instead of 'or')
The rest of the LLVM project uses them and the code should be consistent.

45 ↗

(On Diff #37054)

No need for the nested anyOf(). You are already in an anyOf() context.

46 ↗

(On Diff #37054)

Use isInTemplateInstantiation() instead of 'hasAncestor(functionDecl(ast_matchers::isTemplateInstantiation()))'

50 ↗

(On Diff #37054)

wouldn't the cxxBoolLiteral() also pass the isBoolType() check?

89 ↗

(On Diff #37054)

What about removing from unary_not->getLocStart() until unary_not->getSubExpr()->getLocStart()?
Or if you just want to remove one token do
CharSourceRange::getTokenRange(unary_not->getLocStart(),unary_not->getLocStart()).

101 ↗

(On Diff #37054)

You also need to check for CXXOperatorCallExpr, but only for operators that require the parens.

112 ↗

(On Diff #37054)

I think it is better if we construct a prefix and a suffix strings and then add a single hint for each.
Having multiple insert hints in the same location is brittle.

136 ↗

(On Diff #37054)

There are more literals. For example, NULL (GNUNullExpr)

146 ↗

(On Diff #37054)

Why are you using the _or_null overloads everywhere?
Do you expect them to be null?

153 ↗

(On Diff #37054)

You can move the declaration into the if statement. It doesn't pollute the outer scope and allows you to have simpler variable names.
Eg:

if (const auto *FL = llvm::dyn_cast<FloatingLiteral>(LiteralExpression)) {
  return ...
}

166 ↗

(On Diff #37054)

use isa<>. You don't really need the pointer.

171 ↗

(On Diff #37054)

Use llvm_unreachable() instead

178 ↗

(On Diff #37054)

Just CastExpression->getLocEnd()
You are using getTokenRange so it already covers from the start of the start token to the end of the end token.

198 ↗

(On Diff #37054)

Construct a string and collapse all the insertions into a single one.

223 ↗

(On Diff #37054)

You might also want to add a case for pointers here.
'false' converts to pointer.
ie this is "valid" code:

bool is_null(void* p) { return p == false; }

231 ↗

(On Diff #37054)

just getLocEnd(), same as above.

254 ↗

(On Diff #37054)

isn't this createImplicitCastFromBoolMatcher() ?
if not it is very close.
Maybe you can modify createImplicitCastFromBoolMatcher to reuse it. It is only used once so no reason for it to exist otherwise.

255 ↗

(On Diff #37054)

remove anyOf()

262 ↗

(On Diff #37054)

why this anyOf()? It must have a parent stmt.

274 ↗

(On Diff #37054)

Instead of the custom hasOpcode(BO_EQ) you can use the provided hasOperatorName("==")

285 ↗

(On Diff #37054)

move declarations into if statement. same as above

docs/clang-tidy/checks/readability-implicit-bool-cast.rst

57 ↗

(On Diff #37054)

why do we prefer a static_cast<> here, but we use != 0 in the if() above?

test/clang-tidy/readability-implicit-bool-cast.cpp

3 ↗

(On Diff #37054)

You could have a single function template:
template <typename T>
void functionTaking(T);

Then call with explicit template type:

functionTaking<int>(boolean);

Might make the code below more explicit as you can see the real C++ type is the expression.

22 ↗

(On Diff #37054)

move the // CHECK* comments after the line you are checking. That is the convention on other tests.

22 ↗

(On Diff #37054)

The first check should have the full message, including the [name-of-check] suffix to make sure it is generated correctly.

160 ↗

(On Diff #37054)

// namespace ignoreImplicitCastFromBoolInTemplateInstantiations

328 ↗

(On Diff #37054)

What would happen with code like:

assert(false && "some message");

?

Will this rewrite it to:

assert(false && true);

?

367 ↗

(On Diff #37054)

// namespace ignoreImplicitCastToBoolInTemplateInstantiations

380 ↗

(On Diff #37054)

// namespace ignoreUserDefinedConversionOperator

[piotrdz]

@sbenza: Thanks for the review. I have added my comments with expanations as necessary, and I have already fixed some issues you reported. I'm not posting a new diff at this time, as I want to finish with remaining issues, and then post updated diff once I'm ready with everything. I'll also work on adding a config option to conditionally allow for pointer to bool casts, as discussed on cfe-dev mailing list.

clang-tidy/readability/ImplicitBoolCastCheck.cpp

32 ↗

(On Diff #37054)

I agree. The only question is, should I do this as part of this patch, or isn't it better to check in this code first, and later move the matchers to clang in separate commit? A separate problem is that I don't have commit rights myself, so in any case, I must ask someone else to do it for me.

89 ↗

(On Diff #37054)

Ah, so that's how I do it.

I must say that it is totally not obvious how to work with source ranges in this or similar situations.

I came up with this version with getting token location and "magic" -1, because this is the first thing that actually worked for me. Previously, I was banging my head against the wall, trying to use something obvious like CharSourceRange::getTokenRange(ParentStatement->getLocStart(), ParentStatement->getLocEnd()). Needless to say, it doesn't work as you would expect.

101 ↗

(On Diff #37054)

And I thought I had everything covered. You're right, I'll need to fix that.

136 ↗

(On Diff #37054)

Fair point. Currently NULL will not work also because it's seen as macro, and so eliminated by my unless(isMacroExpansion()). That case (e.g. functionTakingBool(NULL) is actually a bit weird, because the whole ImplicitCastExpr is marked as expanded from NULL macro, and not where it actually occurs in code.

I'll try fixing it later, but I don't relish this. Macros are just evil.

146 ↗

(On Diff #37054)

It's just a habit I picked up early on when working with Clang. I ran into too many crashes due to null pointers, so I decided to always write defensive code. I guess now that I know more about ins and outs of Clang, I can do without it.

153 ↗

(On Diff #37054)

Ah, as irony would have it, it's an example of an implicit bool cast which my check would warn against. But as they say, for every rule, there is an exception.

223 ↗

(On Diff #37054)

No, it doesn't work like that. false will decay to integer constant and result in compile error of trying to compare pointer to an integer:

/tmp/test.cpp:1:35: error: comparison between pointer and integer ('void *' and 'int') [clang-diagnostic-error]
bool test(void* ptr) { return ptr == false; }

At least this we can be sure of - booleans will not convert to pointers, thank heavens for that.

254 ↗

(On Diff #37054)

This is matcher for implicit cast *to* bool, not *from* bool. They only seem to be similar. The only common part is this unless() part, which I extracted to function createExceptionCasesMatcher().

262 ↗

(On Diff #37054)

It doesn't have to be a Stmt. Consider: int integer = false;. Here, the parent of ImplicitCastExpr is a VarDecl.

docs/clang-tidy/checks/readability-implicit-bool-cast.rst

57 ↗

(On Diff #37054)

I can change it to ternary operator but it still looks ugly. Unfortunately, we run here into the same problem as in any other cast - it's just ugly code which pretends to be pretty.

The case of casting from something to bool type is easier to deal with, as such expression always corresponds to some kind of logical comparison. This check aims to make this comparison visible in code, rather than hidden, even under explicit cast like static_cast<bool>. So the choice of comparison operators comes naturally, and in my humble opinion, actually looks better in code than ugly static_cast.

But in the case of casting from bool to other type, we are casting away types, hoping that the conversion happens "out of the box". There just isn't a clear way of representing this in code, other than static_cast<type>. Okay, we can propose here a ternary operator as alternative: (cond) ? 1 : 0, but I feel that this is a bit too verbose, and actually less readable than the static_cast version. Besides, I'm not 100% sure, but I think it may actually change semantics (i.e. add branch to the output code where there wasn't any before). Casts such as these are just ugly and there is little we can do about that :-(.

I don't remember where, but I once read an interesting passage on the topic of casts. Quoting from memory:
"static_cast looks ugly in code, and it looks ugly on purpose. It is an indication that the programmer is trying to outsmart the type system in some way".

test/clang-tidy/readability-implicit-bool-cast.cpp

328 ↗

(On Diff #37054)

No, currently all macros are ignored, and personally, I think it is for the better. I simply don't like messing with macros, as that can have all sort of nasty consequences.

But even if the check would perform such replacement, I have to ask: is it really the fault of this check? Using assert() in such way is a prime example of "hacked" solution to a missing functionality (adding message displayed with the assert).

It would work perfectly, if it was used in a bit more sane manner, such as:

#define assert_with_message(condition, message) assert(condition && message)

assert_with_message(false, "some message");

Then, the implicit cast in macro body would be ignored, and the actual assert statement written in such way would be clearer.

[piotrdz]

I fixed the issues found in review and added a few other enhancements:

• add config options to allow integer and pointer casts in conditionals, e.g. "if (pointer)",
• add support for NULL macro cast to boolean,
• ignore usage of variable declaration in control statements, e.g. "if (int x = function())",
• add support case of false literal to pointer conversion in pre-C++11 standards
• propose NULL instead of nullptr in replacements in pre-C++11 standards

[piotrdz]

I think this is a more complete version, and we're getting closer to getting my patch committed. @sbenza: Do you agree?

clang-tidy/readability/ImplicitBoolCastCheck.cpp

153 ↗

(On Diff #37054)

Also, it turns out, I missed this case in my tests. I added appropriate exclusion for using variable declarations in this way.

223 ↗

(On Diff #37054)

Ah, my bad. clang-tidy tests use by default C++11. Of course this code will be accepted in older C++ standards. I added provisions for that case, and also changed nullptr to NULL in suggested replacements when nullptr is not available.

[sbenza]

clang-tidy/readability/ImplicitBoolCastCheck.cpp

89 ↗

(On Diff #37054)

A little explanation...
When you get getLocStart/getLocEnd you get the location of the start of the first token and the start of the last token.

Eg: in 'foo + bar' getLocStart points to 'f' and getLocEnd points to 'b'.

When you create a CharSourceRange you have two options:

• getTokenRange(): spans from the first char of the token at start to past-the-end of the token at end
• getCharRange(): spans the locations passed exactly.

getTokenRange(loc at 'f', loc at 'b') will give a range spanning 'foo + bar'
getCharRange(loc at 'f', loc at 'b') will give a range spanning 'foo + '

They are both useful. Sometimes you need a little extra help from the lexer of manually offsetting the locations.

Note that if the node spans only one token, then start/end will be the same location.

136 ↗

(On Diff #37054)

Macros are the nemesis of clang-tidy checks...

33 ↗

(On Diff #37707)

I can submit the matchers for you.
I'll do that when the change is stable.

43 ↗

(On Diff #37707)

I was a little confused by the name. I thought the expansion was somehow a null expansion, not that it is the NULL macro.
Maybe should be isNULLMacroExpansion?

87 ↗

(On Diff #37707)

NULL might need an #include

119 ↗

(On Diff #37707)

fold declaration into the if() condition

docs/clang-tidy/checks/readability-implicit-bool-cast.rst

57 ↗

(On Diff #37054)

I agree with all that, but that is not what the comment is saying.
As you say, we want to say 'integer != 0' and not 'static_cast<bool>(integer)'

test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp

20 ↗

(On Diff #37707)

Don't add the [readability...] on each hit. Just the first one.

test/clang-tidy/readability-implicit-bool-cast.cpp

306 ↗

(On Diff #37707)

Just out of curiosity, does it handle -0.0 correctly?

328 ↗

(On Diff #37054)

Is there any non-macro assert-like API where people do the same trick?

[piotrdz]

Addressed latest review issues.

[piotrdz]

There you go, third revision. I think this version should be ready for commit.

clang-tidy/readability/ImplicitBoolCastCheck.cpp

87 ↗

(On Diff #37707)

I changed it to 0.

90 ↗

(On Diff #37707)

Thanks for explaining that, it makes sense to me now. Perhaps such description could be added to Doxygen documentation?

test/clang-tidy/readability-implicit-bool-cast.cpp

306 ↗

(On Diff #37707)

Yes, it will. -0.0 is parsed as expression using binary operator (operator unary minus + double const). It will be turned into (-0.0) != 0.0 which will evaluate to false, the same as original -0.0.

[sbenza]

docs/clang-tidy/checks/readability-implicit-bool-cast.rst

58 ↗

(On Diff #37707)

This is still a thing.
The replacement is not what this comment says.

test/clang-tidy/readability-implicit-bool-cast.cpp

306 ↗

(On Diff #37707)

This is for literals, so it will try to convert to the bool literal instead of using != 0.0
However, the check above does:

(FloatLit->getValue().bitcastToAPInt() == 0) ? "false" : "true";

and that will return "true" for -0.0, I think.

[piotrdz]

Addressed last issues in review

[piotrdz]

Any other comments?

docs/clang-tidy/checks/readability-implicit-bool-cast.rst

59 ↗

(On Diff #37939)

Sorry, somehow I missed that one. I meant to show casts from bool to int here, so the code is wrong, as you say.

test/clang-tidy/readability-implicit-bool-cast.cpp

307 ↗

(On Diff #37939)

No, it doesn't work like that. As I said:
-0.0 will *never* be parsed as a literal. These are two tokens in source code: unary minus AND a 0.0 floating point literal. The standard says that literals are only numbers and - is never a part of a literal.

However, I can grant that someone, somewhere might invent negative literals, so to be absolutely on the safe side, I changed the code in question to take the absolute value of literal.

And to prove that -0.0 is parsed as an expression, I added tests for that.

Also note that (-0.0) != 0.0 is perfectly fine as a replacement, as it still evaluates to false. The standard says that -0.0 behaves just like regular 0.0 in most aspects, including equality with 0.0 and so on. The only difference is seen in operations like division or multiplication, where it changes the sign of result accordingly.

[sbenza]

test/clang-tidy/readability-implicit-bool-cast.cpp

307 ↗

(On Diff #37939)

You are right.
I was thinking that -0.0 was a FloatingLiteral, but it is in fact a UnaryOperator with a FloatingLiteral inside. Sorry for the noise.

[piotrdz]

Thanks for accepting the review. However, I still have to ask someone to make the commit. Or should I obtain commit rights myself? If so, how do I do that?

[Eugene.Zelenko]

See http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

[piotrdz]

I have obtained now commit rights but I want to make sure: I can commit this patch now that it is accepted? That is, accepting the review is the same as saying "LGTM", right?

[Eugene.Zelenko]

Accepted status is what is most important.

Please don't forget to mention Differential revision in commit comment, so is revision will be closed automatically. See example.

Please also be near computer for ~hour, just to make sure all build bots finished successfully. I learned this after breaking LLDB MSVC builds couple of times :-)

[piotrdz]

@eugene: thanks for clarifying that and your advice. I made the commit and it seems I didn't break anything, yet :-).

clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyReadabilityModule
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp
   IdentifierNamingCheck.cpp
+  ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   SimplifyBooleanExprCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp

clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.h

+//===--- ImplicitBoolCastCheck.h - clang-tidy--------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CAST_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CAST_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Checks for use of implicit bool casts in expressions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-cast.html
+class ImplicitBoolCastCheck : public ClangTidyCheck {
+public:
+  ImplicitBoolCastCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+        AllowConditionalIntegerCasts(
+            Options.get("AllowConditionalIntegerCasts", 0) != 0),
+        AllowConditionalPointerCasts(
+            Options.get("AllowConditionalPointerCasts", 0) != 0) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void handleCastToBool(const ImplicitCastExpr *CastExpression,
+                        const Stmt *ParentStatement, ASTContext &Context);
+  void handleCastFromBool(const ImplicitCastExpr *CastExpression,
+                          const ImplicitCastExpr *FurtherImplicitCastExpression,
+                          ASTContext &Context);
+
+  bool AllowConditionalIntegerCasts;
+  bool AllowConditionalPointerCasts;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CAST_H

clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp

+//===--- ImplicitBoolCastCheck.cpp - clang-tidy----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ImplicitBoolCastCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+namespace {
+
+const internal::VariadicDynCastAllOfMatcher<Stmt, ParenExpr> parenExpr;
+
+AST_MATCHER_P(CastExpr, hasCastKind, CastKind, Kind) {
+  return Node.getCastKind() == Kind;
+}
+
+AST_MATCHER(QualType, isBool) {
+  return !Node.isNull() && Node->isBooleanType();
+}
+
+AST_MATCHER(Stmt, isMacroExpansion) {
+  SourceManager &SM = Finder->getASTContext().getSourceManager();
+  SourceLocation Loc = Node.getLocStart();
+  return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc);
+}
+
+bool isNULLMacroExpansion(const Stmt *Statement, ASTContext &Context) {
+  SourceManager &SM = Context.getSourceManager();
+  const LangOptions &LO = Context.getLangOpts();
+  SourceLocation Loc = Statement->getLocStart();
+  return SM.isMacroBodyExpansion(Loc) &&
+         clang::Lexer::getImmediateMacroName(Loc, SM, LO) == "NULL";
+}
+
+AST_MATCHER(Stmt, isNULLMacroExpansion) {
+  return isNULLMacroExpansion(&Node, Finder->getASTContext());
+}
+
+ast_matchers::internal::Matcher<Expr> createExceptionCasesMatcher() {
+  return expr(anyOf(hasParent(explicitCastExpr()),
+                    allOf(isMacroExpansion(), unless(isNULLMacroExpansion())),
+                    isInTemplateInstantiation(),
+                    hasAncestor(functionTemplateDecl())));
+}
+
+StatementMatcher createImplicitCastFromBoolMatcher() {
+  return implicitCastExpr(
+      unless(createExceptionCasesMatcher()),
+      anyOf(hasCastKind(CK_IntegralCast), hasCastKind(CK_IntegralToFloating),
+            // Prior to C++11 cast from bool literal to pointer was allowed.
+            allOf(anyOf(hasCastKind(CK_NullToPointer),
+                        hasCastKind(CK_NullToMemberPointer)),
+                  hasSourceExpression(cxxBoolLiteral()))),
+      hasSourceExpression(expr(hasType(qualType(isBool())))));
+}
+
+StringRef
+getZeroLiteralToCompareWithForGivenType(CastKind CastExpressionKind,
+                                        QualType CastSubExpressionType,
+                                        ASTContext &Context) {
+  switch (CastExpressionKind) {
+  case CK_IntegralToBoolean:
+    return CastSubExpressionType->isUnsignedIntegerType() ? "0u" : "0";
+
+  case CK_FloatingToBoolean:
+    return Context.hasSameType(CastSubExpressionType, Context.FloatTy) ? "0.0f"
+                                                                       : "0.0";
+
+  case CK_PointerToBoolean:
+  case CK_MemberPointerToBoolean: // Fall-through on purpose.
+    return Context.getLangOpts().CPlusPlus11 ? "nullptr" : "0";
+
+  default:
+    assert(false && "Unexpected cast kind");
+  }
+}
+
+bool isUnaryLogicalNotOperator(const Stmt *Statement) {
+  const auto *UnaryOperatorExpression =
+      llvm::dyn_cast<UnaryOperator>(Statement);
+  return UnaryOperatorExpression != nullptr &&
+         UnaryOperatorExpression->getOpcode() == UO_LNot;
+}
+
+bool areParensNeededForOverloadedOperator(OverloadedOperatorKind OperatorKind) {
+  switch (OperatorKind) {
+  case OO_New:
+  case OO_Delete: // Fall-through on purpose.
+  case OO_Array_New:
+  case OO_Array_Delete:
+  case OO_ArrowStar:
+  case OO_Arrow:
+  case OO_Call:
+  case OO_Subscript:
+    return false;
+
+  default:
+    return true;
+  }
+}
+
+bool areParensNeededForStatement(const Stmt *Statement) {
+  if (const CXXOperatorCallExpr *OverloadedOperatorCall =
+          llvm::dyn_cast<CXXOperatorCallExpr>(Statement)) {
+    return areParensNeededForOverloadedOperator(
+        OverloadedOperatorCall->getOperator());
+  }
+
+  return llvm::isa<BinaryOperator>(Statement) ||
+         llvm::isa<UnaryOperator>(Statement);
+}
+
+void addFixItHintsForGenericExpressionCastToBool(
+    DiagnosticBuilder &Diagnostic, const ImplicitCastExpr *CastExpression,
+    const Stmt *ParentStatement, ASTContext &Context) {
+  // In case of expressions like (! integer), we should remove the redundant not
+  // operator and use inverted comparison (integer == 0).
+  bool InvertComparison =
+      ParentStatement != nullptr && isUnaryLogicalNotOperator(ParentStatement);
+  if (InvertComparison) {
+    SourceLocation ParentStartLoc = ParentStatement->getLocStart();
+    SourceLocation ParentEndLoc =
+        llvm::cast<UnaryOperator>(ParentStatement)->getSubExpr()->getLocStart();
+    Diagnostic.AddFixItHint(FixItHint::CreateRemoval(
+        CharSourceRange::getCharRange(ParentStartLoc, ParentEndLoc)));
+
+    auto FurtherParents = Context.getParents(*ParentStatement);
+    ParentStatement = FurtherParents[0].get<Stmt>();
+  }
+
+  const Expr *SubExpression = CastExpression->getSubExpr();
+
+  bool NeedInnerParens = areParensNeededForStatement(SubExpression);
+  bool NeedOuterParens = ParentStatement != nullptr &&
+                         areParensNeededForStatement(ParentStatement);
+
+  std::string StartLocInsertion;
+
+  if (NeedOuterParens) {
+    StartLocInsertion += "(";
+  }
+  if (NeedInnerParens) {
+    StartLocInsertion += "(";
+  }
+
+  if (!StartLocInsertion.empty()) {
+    SourceLocation StartLoc = CastExpression->getLocStart();
+    Diagnostic.AddFixItHint(
+        FixItHint::CreateInsertion(StartLoc, StartLocInsertion));
+  }
+
+  std::string EndLocInsertion;
+
+  if (NeedInnerParens) {
+    EndLocInsertion += ")";
+  }
+
+  if (InvertComparison) {
+    EndLocInsertion += " == ";
+  } else {
+    EndLocInsertion += " != ";
+  }
+
+  EndLocInsertion += getZeroLiteralToCompareWithForGivenType(
+      CastExpression->getCastKind(), SubExpression->getType(), Context);
+
+  if (NeedOuterParens) {
+    EndLocInsertion += ")";
+  }
+
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+      CastExpression->getLocEnd(), 0, Context.getSourceManager(),
+      Context.getLangOpts());
+  Diagnostic.AddFixItHint(FixItHint::CreateInsertion(EndLoc, EndLocInsertion));
+}
+
+StringRef getEquivalentBoolLiteralForExpression(const Expr *Expression,
+                                                ASTContext &Context) {
+  if (isNULLMacroExpansion(Expression, Context)) {
+    return "false";
+  }
+
+  if (const auto *IntLit = llvm::dyn_cast<IntegerLiteral>(Expression)) {
+    return (IntLit->getValue() == 0) ? "false" : "true";
+  }
+
+  if (const auto *FloatLit = llvm::dyn_cast<FloatingLiteral>(Expression)) {
+    llvm::APFloat FloatLitAbsValue = FloatLit->getValue();
+    FloatLitAbsValue.clearSign();
+    return (FloatLitAbsValue.bitcastToAPInt() == 0) ? "false" : "true";
+  }
+
+  if (const auto *CharLit = llvm::dyn_cast<CharacterLiteral>(Expression)) {
+    return (CharLit->getValue() == 0) ? "false" : "true";
+  }
+
+  if (llvm::isa<StringLiteral>(Expression->IgnoreCasts())) {
+    return "true";
+  }
+
+  return StringRef();
+}
+
+void addFixItHintsForLiteralCastToBool(DiagnosticBuilder &Diagnostic,
+                                       const ImplicitCastExpr *CastExpression,
+                                       StringRef EquivalentLiteralExpression) {
+  SourceLocation StartLoc = CastExpression->getLocStart();
+  SourceLocation EndLoc = CastExpression->getLocEnd();
+
+  Diagnostic.AddFixItHint(FixItHint::CreateReplacement(
+      CharSourceRange::getTokenRange(StartLoc, EndLoc),
+      EquivalentLiteralExpression));
+}
+
+void addFixItHintsForGenericExpressionCastFromBool(
+    DiagnosticBuilder &Diagnostic, const ImplicitCastExpr *CastExpression,
+    ASTContext &Context, StringRef OtherType) {
+  const Expr *SubExpression = CastExpression->getSubExpr();
+  bool NeedParens = !llvm::isa<ParenExpr>(SubExpression);
+
+  std::string StartLocInsertion = "static_cast<";
+  StartLocInsertion += OtherType.str();
+  StartLocInsertion += ">";
+  if (NeedParens) {
+    StartLocInsertion += "(";
+  }
+
+  SourceLocation StartLoc = CastExpression->getLocStart();
+  Diagnostic.AddFixItHint(
+      FixItHint::CreateInsertion(StartLoc, StartLocInsertion));
+
+  if (NeedParens) {
+    SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+        CastExpression->getLocEnd(), 0, Context.getSourceManager(),
+        Context.getLangOpts());
+
+    Diagnostic.AddFixItHint(FixItHint::CreateInsertion(EndLoc, ")"));
+  }
+}
+
+StringRef getEquivalentLiteralForBoolLiteral(
+    const CXXBoolLiteralExpr *BoolLiteralExpression, QualType DestinationType,
+    ASTContext &Context) {
+  // Prior to C++11, false literal could be implicitly converted to pointer.
+  if (!Context.getLangOpts().CPlusPlus11 &&
+      (DestinationType->isPointerType() ||
+       DestinationType->isMemberPointerType()) &&
+      BoolLiteralExpression->getValue() == false) {
+    return "0";
+  }
+
+  if (DestinationType->isFloatingType()) {
+    if (BoolLiteralExpression->getValue() == true) {
+      return Context.hasSameType(DestinationType, Context.FloatTy) ? "1.0f"
+                                                                   : "1.0";
+    }
+    return Context.hasSameType(DestinationType, Context.FloatTy) ? "0.0f"
+                                                                 : "0.0";
+  }
+
+  if (BoolLiteralExpression->getValue() == true) {
+    return DestinationType->isUnsignedIntegerType() ? "1u" : "1";
+  }
+  return DestinationType->isUnsignedIntegerType() ? "0u" : "0";
+}
+
+void addFixItHintsForLiteralCastFromBool(DiagnosticBuilder &Diagnostic,
+                                         const ImplicitCastExpr *CastExpression,
+                                         ASTContext &Context,
+                                         QualType DestinationType) {
+  SourceLocation StartLoc = CastExpression->getLocStart();
+  SourceLocation EndLoc = CastExpression->getLocEnd();
+  const auto *BoolLiteralExpression =
+      llvm::dyn_cast<CXXBoolLiteralExpr>(CastExpression->getSubExpr());
+
+  Diagnostic.AddFixItHint(FixItHint::CreateReplacement(
+      CharSourceRange::getTokenRange(StartLoc, EndLoc),
+      getEquivalentLiteralForBoolLiteral(BoolLiteralExpression, DestinationType,
+                                         Context)));
+}
+
+StatementMatcher createConditionalExpressionMatcher() {
+  return stmt(anyOf(ifStmt(), conditionalOperator(),
+                    parenExpr(hasParent(conditionalOperator()))));
+}
+
+bool isAllowedConditionalCast(const ImplicitCastExpr *CastExpression,
+                              ASTContext &Context) {
+  auto AllowedConditionalMatcher = stmt(hasParent(stmt(
+      anyOf(createConditionalExpressionMatcher(),
+            unaryOperator(hasOperatorName("!"),
+                          hasParent(createConditionalExpressionMatcher()))))));
+
+  auto MatchResult = match(AllowedConditionalMatcher, *CastExpression, Context);
+  return !MatchResult.empty();
+}
+
+} // anonymous namespace
+
+void ImplicitBoolCastCheck::registerMatchers(MatchFinder *Finder) {
+  // This check doesn't make much sense if we run it on language without
+  // built-in bool support.
+  if (!getLangOpts().Bool) {
+    return;
+  }
+
+  Finder->addMatcher(
+      implicitCastExpr(
+          // Exclude cases common to implicit cast to and from bool.
+          unless(createExceptionCasesMatcher()),
+          // Exclude case of using if or while statements with variable
+          // declaration, e.g.:
+          //   if (int var = functionCall()) {}
+          unless(
+              hasParent(stmt(anyOf(ifStmt(), whileStmt()), has(declStmt())))),
+          anyOf(hasCastKind(CK_IntegralToBoolean),
+                hasCastKind(CK_FloatingToBoolean),
+                hasCastKind(CK_PointerToBoolean),
+                hasCastKind(CK_MemberPointerToBoolean)),
+          // Retrive also parent statement, to check if we need additional
+          // parens in replacement.
+          anyOf(hasParent(stmt().bind("parentStmt")), anything()))
+          .bind("implicitCastToBool"),
+      this);
+
+  Finder->addMatcher(
+      implicitCastExpr(
+          createImplicitCastFromBoolMatcher(),
+          // Exclude comparisons of bools, as they are always cast to integers
+          // in such context:
+          //   bool_expr_a == bool_expr_b
+          //   bool_expr_a != bool_expr_b
+          unless(hasParent(binaryOperator(
+              anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+              hasLHS(createImplicitCastFromBoolMatcher()),
+              hasRHS(createImplicitCastFromBoolMatcher())))),
+          // Check also for nested casts, for example: bool -> int -> float.
+          anyOf(hasParent(implicitCastExpr().bind("furtherImplicitCast")),
+                anything()))
+          .bind("implicitCastFromBool"),
+      this);
+}
+
+void ImplicitBoolCastCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *CastToBool =
+          Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastToBool")) {
+    const auto *ParentStatement = Result.Nodes.getNodeAs<Stmt>("parentStmt");
+    return handleCastToBool(CastToBool, ParentStatement, *Result.Context);
+  }
+
+  if (const auto *CastFromBool =
+          Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastFromBool")) {
+    const auto *FurtherImplicitCastExpression =
+        Result.Nodes.getNodeAs<ImplicitCastExpr>("furtherImplicitCast");
+    return handleCastFromBool(CastFromBool, FurtherImplicitCastExpression,
+                              *Result.Context);
+  }
+}
+
+void ImplicitBoolCastCheck::handleCastToBool(
+    const ImplicitCastExpr *CastExpression, const Stmt *ParentStatement,
+    ASTContext &Context) {
+  if (AllowConditionalPointerCasts &&
+      (CastExpression->getCastKind() == CK_PointerToBoolean ||
+       CastExpression->getCastKind() == CK_MemberPointerToBoolean) &&
+      isAllowedConditionalCast(CastExpression, Context)) {
+    return;
+  }
+
+  if (AllowConditionalIntegerCasts &&
+      CastExpression->getCastKind() == CK_IntegralToBoolean &&
+      isAllowedConditionalCast(CastExpression, Context)) {
+    return;
+  }
+
+  std::string OtherType = CastExpression->getSubExpr()->getType().getAsString();
+  DiagnosticBuilder Diagnostic =
+      diag(CastExpression->getLocStart(), "implicit cast '%0' -> bool")
+      << OtherType;
+
+  StringRef EquivalentLiteralExpression = getEquivalentBoolLiteralForExpression(
+      CastExpression->getSubExpr(), Context);
+  if (!EquivalentLiteralExpression.empty()) {
+    addFixItHintsForLiteralCastToBool(Diagnostic, CastExpression,
+                                      EquivalentLiteralExpression);
+  } else {
+    addFixItHintsForGenericExpressionCastToBool(Diagnostic, CastExpression,
+                                                ParentStatement, Context);
+  }
+}
+
+void ImplicitBoolCastCheck::handleCastFromBool(
+    const ImplicitCastExpr *CastExpression,
+    const ImplicitCastExpr *FurtherImplicitCastExpression,
+    ASTContext &Context) {
+  QualType DestinationType = (FurtherImplicitCastExpression != nullptr)
+                                 ? FurtherImplicitCastExpression->getType()
+                                 : CastExpression->getType();
+  std::string DestinationTypeString = DestinationType.getAsString();
+  DiagnosticBuilder Diagnostic =
+      diag(CastExpression->getLocStart(), "implicit cast bool -> '%0'")
+      << DestinationTypeString;
+
+  if (llvm::isa<CXXBoolLiteralExpr>(CastExpression->getSubExpr())) {
+    addFixItHintsForLiteralCastFromBool(Diagnostic, CastExpression, Context,
+                                        DestinationType);
+  } else {
+    addFixItHintsForGenericExpressionCastFromBool(
+        Diagnostic, CastExpression, Context, DestinationTypeString);
+  }
+}
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
 #include "IdentifierNamingCheck.h"
+#include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<ElseAfterReturnCheck>(
         "readability-else-after-return");
     CheckFactories.registerCheck<FunctionSizeCheck>(
         "readability-function-size");
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");
+    CheckFactories.registerCheck<ImplicitBoolCastCheck>(
+        "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    modernize-use-default
    modernize-use-nullptr
    modernize-use-override
    readability-braces-around-statements
    readability-container-size-empty
    readability-else-after-return
    readability-function-size
    readability-identifier-naming
+   readability-implicit-bool-cast
    readability-inconsistent-declaration-parameter-name
    readability-named-parameter
    readability-redundant-smartptr-get
    readability-redundant-string-cstr
    readability-simplify-boolean-expr
    readability-uniqueptr-delete-release

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-cast.rst

+readability-implicit-bool-cast
+==============================
+
+This check can be used to find implicit conversions between built-in types and
+booleans. Depending on use case, it may simply help with readability of the code,
+or in some cases, point to potential bugs which remain unnoticed due to implicit
+conversions.
+
+The following is a real-world example of bug which was hiding behind implicit
+bool cast:
+
+.. code:: c++
+
+  class Foo {
+    int m_foo;
+  public:
+    void setFoo(bool foo) { m_foo = foo; } // warning: implicit cast bool -> int
+    int getFoo() { return m_foo; }
+  };
+
+  void use(Foo& foo) {
+    bool value = foo.getFoo(); // warning: implicit cast int -> bool
+  }
+
+This code is the result of unsuccessful refactoring, where type of ``m_foo``
+changed from ``bool`` to ``int``. The programmer forgot to change all
+occurences of ``bool``, and the remaining code is no longer correct, yet it
+still compiles without any visible warnings.
+
+In addition to issuing warnings, FixIt hints are provided to help solve
+the reported issues. This can be used for improving readabilty of code, for example:
+
+.. code:: c++
+
+  void conversionsToBool() {
+    float floating;
+    bool boolean = floating;
+    // ^ propose replacement: bool boolean = floating != 0.0f;
+
+    int integer;
+    if (integer) {}
+    // ^ propose replacement: if (integer != 0) {}
+
+    int* pointer;
+    if (!pointer) {}
+    // ^ propose replacement: if (pointer == nullptr) {}
+
+    while (1) {}
+    // ^ propose replacement: while (true) {}
+  }
+
+  void functionTakingInt(int param);
+
+  void conversionsFromBool() {
+    bool boolean;
+    functionTakingInt(boolean);
+    // ^ propose replacement: functionTakingInt(static_cast<int>(boolean));
+
+    functionTakingInt(true);
+    // ^ propose replacement: functionTakingInt(1);
+  }
+
+In general, the following cast types are checked:
+ - integer expression/literal to boolean,
+ - floating expression/literal to boolean,
+ - pointer/pointer to member/``nullptr``/``NULL`` to boolean,
+ - boolean expression/literal to integer,
+ - boolean expression/literal to floating.
+
+The rules for generating FixIt hints are:
+ - in case of casts from other built-in type to bool, an explicit comparison
+   is proposed to make it clear what exaclty is being compared:
+
+   - ``bool boolean = floating;`` is changed to ``bool boolean = floating == 0.0f;``,
+   - for other types, appropriate literals are used (``0``, ``0u``, ``0.0f``, ``0.0``, ``nullptr``),
+ - in case of negated expressions cast to bool, the proposed replacement with
+   comparison is simplified:
+
+   - ``if (!pointer)`` is changed to ``if (pointer == nullptr)``,
+ - in case of casts from bool to other built-in types, an explicit ``static_cast``
+   is proposed to make it clear that a cast is taking place:
+
+   - ``int integer = boolean;`` is changed to ``int integer = static_cast<int>(boolean);``,
+ - if the cast is performed on type literals, an equivalent literal is proposed,
+   according to what type is actually expected, for example:
+
+   - ``functionTakingBool(0);`` is changed to ``functionTakingBool(false);``,
+   - ``functionTakingInt(true);`` is changed to ``functionTakingInt(1);``,
+   - for other types, appropriate literals are used (``false``, ``true``, ``0``, ``1``, ``0u``, ``1u``,
+     ``0.0f``, ``1.0f``, ``0.0``, ``1.0f``).
+
+Some additional accommodations are made for pre-C++11 dialects:
+ - ``false`` literal cast to pointer is detected,
+ - instead of ``nullptr`` literal, ``0`` is proposed as replacement.
+
+Occurences of implicit casts inside macros and template instantiations are
+deliberately ignored, as it is not clear how to deal with such cases.

clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp

+// RUN: %check_clang_tidy %s readability-implicit-bool-cast %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-implicit-bool-cast.AllowConditionalIntegerCasts, value: 1}, \
+// RUN:   {key: readability-implicit-bool-cast.AllowConditionalPointerCasts, value: 1}]}' \
+// RUN: -- -std=c++11
+
+template<typename T>
+void functionTaking(T);
+
+int functionReturningInt();
+int* functionReturningPointer();
+
+struct Struct {
+  int member;
+};
+
+
+void regularImplicitCastIntegerToBoolIsNotIgnored() {
+  int integer = 0;
+  functionTaking<bool>(integer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool [readability-implicit-bool-cast]
+  // CHECK-FIXES: functionTaking<bool>(integer != 0);
+}
+
+void implicitCastIntegerToBoolInConditionalsIsAllowed() {
+  if (functionReturningInt()) {}
+  if (!functionReturningInt()) {}
+  int value1 = functionReturningInt() ? 1 : 2;
+  int value2 = ! functionReturningInt() ? 1 : 2;
+}
+
+void regularImplicitCastPointerToBoolIsNotIgnored() {
+  int* pointer = nullptr;
+  functionTaking<bool>(pointer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int *' -> bool
+  // CHECK-FIXES: functionTaking<bool>(pointer != nullptr);
+
+  int Struct::* memberPointer = &Struct::member;
+  functionTaking<bool>(memberPointer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int struct Struct::*' -> bool
+  // CHECK-FIXES: functionTaking<bool>(memberPointer != nullptr);
+}
+
+void implicitCastPointerToBoolInConditionalsIsAllowed() {
+  if (functionReturningPointer()) {}
+  if (not functionReturningPointer()) {}
+  int value1 = functionReturningPointer() ? 1 : 2;
+  int value2 = (not functionReturningPointer()) ? 1 : 2;
+
+  int Struct::* memberPointer = &Struct::member;
+  if (memberPointer) {}
+  if (memberPointer) {}
+  int value3 = memberPointer ? 1 : 2;
+  int value4 = (not memberPointer) ? 1 : 2;
+}

clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp

+// RUN: clang-tidy %s -checks=-*,readability-implicit-bool-cast -- -std=c++98
+
+#include <cstddef> // for NULL
+
+template<typename T>
+void functionTaking(T);
+
+struct Struct {
+  int member;
+};
+
+void useOldNullMacroInReplacements() {
+  int* pointer = NULL;
+  functionTaking<bool>(pointer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int *' -> bool [readability-implicit-bool-cast]
+  // CHECK-FIXES: functionTaking<bool>(pointer != 0);
+
+  int Struct::* memberPointer = NULL;
+  functionTaking<bool>(!memberPointer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast 'int struct Struct::*' -> bool
+  // CHECK-FIXES: functionTaking<bool>(memberPointer == 0);
+}
+
+void fixFalseLiteralConvertingToNullPointer() {
+  functionTaking<int*>(false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int *'
+  // CHECK-FIXES: functionTaking<int*>(0);
+
+  int* pointer = NULL;
+  if (pointer == false) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit cast bool -> 'int *'
+  // CHECK-FIXES: if (pointer == 0) {}
+
+  functionTaking<int Struct::*>(false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'int struct Struct::*'
+  // CHECK-FIXES: functionTaking<int Struct::*>(0);
+
+  int Struct::* memberPointer = NULL;
+  if (memberPointer != false) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int struct Struct::*'
+  // CHECK-FIXES: if (memberPointer != 0) {}
+}

clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp

+// RUN: %check_clang_tidy %s readability-implicit-bool-cast %t
+
+#include <cstddef> // for NULL
+
+template<typename T>
+void functionTaking(T);
+
+struct Struct {
+  int member;
+};
+
+
+////////// Implicit cast from bool.
+
+void implicitCastFromBoolSimpleCases() {
+  bool boolean = true;
+
+  functionTaking<bool>(boolean);
+
+  functionTaking<int>(boolean);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit cast bool -> 'int' [readability-implicit-bool-cast]
+  // CHECK-FIXES: functionTaking<int>(static_cast<int>(boolean));
+
+  functionTaking<unsigned long>(boolean);
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'unsigned long'
+  // CHECK-FIXES: functionTaking<unsigned long>(static_cast<unsigned long>(boolean));
+
+  functionTaking<char>(boolean);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'char'
+  // CHECK-FIXES: functionTaking<char>(static_cast<char>(boolean));
+
+  functionTaking<float>(boolean);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast bool -> 'float'
+  // CHECK-FIXES: functionTaking<float>(static_cast<float>(boolean));
+
+  functionTaking<double>(boolean);
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit cast bool -> 'double'
+  // CHECK-FIXES: functionTaking<double>(static_cast<double>(boolean));
+}
+
+float implicitCastFromBoolInReturnValue() {
+  bool boolean = false;
+  return boolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit cast bool -> 'float'
+  // CHECK-FIXES: return static_cast<float>(boolean);
+}
+
+void implicitCastFromBoolInSingleBoolExpressions() {
+  bool boolean = true;
+
+  int integer = boolean - 3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit cast bool -> 'int'
+  // CHECK-FIXES: int integer = static_cast<int>(boolean) - 3;
+
+  float floating = boolean / 0.3f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit cast bool -> 'float'
+  // CHECK-FIXES: float floating = static_cast<float>(boolean) / 0.3f;
+
+  char character = boolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit cast bool -> 'char'
+  // CHECK-FIXES: char character = static_cast<char>(boolean);
+}
+
+void implicitCastFromBoollInComplexBoolExpressions() {
+  bool boolean = true;
+  bool anotherBoolean = false;
+
+  int integer = boolean && anotherBoolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit cast bool -> 'int'
+  // CHECK-FIXES: int integer = static_cast<int>(boolean && anotherBoolean);
+
+  unsigned long unsignedLong = (! boolean) + 4ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit cast bool -> 'unsigned long'
+  // CHECK-FIXES: unsigned long unsignedLong = static_cast<unsigned long>(! boolean) + 4ul;
+
+  float floating = (boolean || anotherBoolean) * 0.3f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit cast bool -> 'float'
+  // CHECK-FIXES: float floating = static_cast<float>(boolean || anotherBoolean) * 0.3f;
+
+  double doubleFloating = (boolean && (anotherBoolean || boolean)) * 0.3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit cast bool -> 'double'
+  // CHECK-FIXES: double doubleFloating = static_cast<double>(boolean && (anotherBoolean || boolean)) * 0.3;
+}
+
+void implicitCastFromBoolLiterals() {
+  functionTaking<int>(true);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit cast bool -> 'int'
+  // CHECK-FIXES: functionTaking<int>(1);
+
+  functionTaking<unsigned long>(false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'unsigned long'
+  // CHECK-FIXES: functionTaking<unsigned long>(0u);
+
+  functionTaking<char>(true);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'char'
+  // CHECK-FIXES: functionTaking<char>(1);
+
+  functionTaking<float>(false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast bool -> 'float'
+  // CHECK-FIXES: functionTaking<float>(0.0f);
+
+  functionTaking<double>(true);
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit cast bool -> 'double'
+  // CHECK-FIXES: functionTaking<double>(1.0);
+}
+
+void implicitCastFromBoolInComparisons() {
+  bool boolean = true;
+  int integer = 0;
+
+  functionTaking<bool>(boolean == integer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int'
+  // CHECK-FIXES: functionTaking<bool>(static_cast<int>(boolean) == integer);
+
+  functionTaking<bool>(integer != boolean);
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: implicit cast bool -> 'int'
+  // CHECK-FIXES: functionTaking<bool>(integer != static_cast<int>(boolean));
+}
+
+void ignoreBoolComparisons() {
+  bool boolean = true;
+  bool anotherBoolean = false;
+
+  functionTaking<bool>(boolean == anotherBoolean);
+  functionTaking<bool>(boolean != anotherBoolean);
+}
+
+void ignoreExplicitCastsFromBool() {
+  bool boolean = true;
+
+  int integer = static_cast<int>(boolean) + 3;
+  float floating = static_cast<float>(boolean) * 0.3f;
+  char character = static_cast<char>(boolean);
+}
+
+void ignoreImplicitCastFromBoolInMacroExpansions() {
+  bool boolean = true;
+
+  #define CAST_FROM_BOOL_IN_MACRO_BODY boolean + 3
+  int integerFromMacroBody = CAST_FROM_BOOL_IN_MACRO_BODY;
+
+  #define CAST_FROM_BOOL_IN_MACRO_ARGUMENT(x) x + 3
+  int integerFromMacroArgument = CAST_FROM_BOOL_IN_MACRO_ARGUMENT(boolean);
+}
+
+namespace ignoreImplicitCastFromBoolInTemplateInstantiations {
+
+template<typename T>
+void templateFunction() {
+  bool boolean = true;
+  T uknownType = boolean + 3;
+}
+
+void useOfTemplateFunction() {
+  templateFunction<int>();
+}
+
+} // namespace ignoreImplicitCastFromBoolInTemplateInstantiations
+
+////////// Implicit cast to bool.
+
+void implicitCastToBoolSimpleCases() {
+  int integer = 10;
+  functionTaking<bool>(integer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: functionTaking<bool>(integer != 0);
+
+  unsigned long unsignedLong = 10;
+  functionTaking<bool>(unsignedLong);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'unsigned long' -> bool
+  // CHECK-FIXES: functionTaking<bool>(unsignedLong != 0u);
+
+  float floating = 0.0f;
+  functionTaking<bool>(floating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: functionTaking<bool>(floating != 0.0f);
+
+  double doubleFloating = 1.0f;
+  functionTaking<bool>(doubleFloating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'double' -> bool
+  // CHECK-FIXES: functionTaking<bool>(doubleFloating != 0.0);
+
+  char character = 'a';
+  functionTaking<bool>(character);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'char' -> bool
+  // CHECK-FIXES: functionTaking<bool>(character != 0);
+
+  int* pointer = nullptr;
+  functionTaking<bool>(pointer);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int *' -> bool
+  // CHECK-FIXES: functionTaking<bool>(pointer != nullptr);
+
+  auto pointerToMember = &Struct::member;
+  functionTaking<bool>(pointerToMember);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int struct Struct::*' -> bool
+  // CHECK-FIXES: functionTaking<bool>(pointerToMember != nullptr);
+}
+
+void implicitCastToBoolInSingleExpressions() {
+  int integer = 10;
+  bool boolComingFromInt = integer;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: bool boolComingFromInt = integer != 0;
+
+  float floating = 10.0f;
+  bool boolComingFromFloat = floating;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: bool boolComingFromFloat = floating != 0.0f;
+
+  char character = 'a';
+  bool boolComingFromChar = character;
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: implicit cast 'char' -> bool
+  // CHECK-FIXES: bool boolComingFromChar = character != 0;
+
+  int* pointer = nullptr;
+  bool boolComingFromPointer = pointer;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit cast 'int *' -> bool
+  // CHECK-FIXES: bool boolComingFromPointer = pointer != nullptr;
+}
+
+void implicitCastToBoolInComplexExpressions() {
+  bool boolean = true;
+
+  int integer = 10;
+  int anotherInteger = 20;
+  bool boolComingFromInteger = integer + anotherInteger;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: bool boolComingFromInteger = (integer + anotherInteger) != 0;
+
+  float floating = 0.2f;
+  bool boolComingFromFloating = floating - 0.3f || boolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: bool boolComingFromFloating = ((floating - 0.3f) != 0.0f) || boolean;
+
+  double doubleFloating = 0.3;
+  bool boolComingFromDoubleFloating = (doubleFloating - 0.4) && boolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit cast 'double' -> bool
+  // CHECK-FIXES: bool boolComingFromDoubleFloating = ((doubleFloating - 0.4) != 0.0) && boolean;
+}
+
+void implicitCastInNegationExpressions() {
+  int integer = 10;
+  bool boolComingFromNegatedInt = !integer;
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: bool boolComingFromNegatedInt = integer == 0;
+
+  float floating = 10.0f;
+  bool boolComingFromNegatedFloat = ! floating;
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: bool boolComingFromNegatedFloat = floating == 0.0f;
+
+  char character = 'a';
+  bool boolComingFromNegatedChar = (! character);
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit cast 'char' -> bool
+  // CHECK-FIXES: bool boolComingFromNegatedChar = (character == 0);
+
+  int* pointer = nullptr;
+  bool boolComingFromNegatedPointer = not pointer;
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: implicit cast 'int *' -> bool
+  // CHECK-FIXES: bool boolComingFromNegatedPointer = pointer == nullptr;
+}
+
+void implicitCastToBoolInControlStatements() {
+  int integer = 10;
+  if (integer) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: if (integer != 0) {}
+
+  long int longInteger = 0.2f;
+  for (;longInteger;) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit cast 'long' -> bool
+  // CHECK-FIXES: for (;longInteger != 0;) {}
+
+  float floating = 0.3f;
+  while (floating) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: while (floating != 0.0f) {}
+
+  double doubleFloating = 0.4;
+  do {} while (doubleFloating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicit cast 'double' -> bool
+  // CHECK-FIXES: do {} while (doubleFloating != 0.0);
+}
+
+bool implicitCastToBoolInReturnValue() {
+  float floating = 1.0f;
+  return floating;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: return floating != 0.0f;
+}
+
+void implicitCastToBoolFromLiterals() {
+  functionTaking<bool>(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: functionTaking<bool>(false);
+
+  functionTaking<bool>(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+  functionTaking<bool>(2ul);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'unsigned long' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+
+  functionTaking<bool>(0.0f);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: functionTaking<bool>(false);
+
+  functionTaking<bool>(1.0f);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+  functionTaking<bool>(2.0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'double' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+
+  functionTaking<bool>('\0');
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'char' -> bool
+  // CHECK-FIXES: functionTaking<bool>(false);
+
+  functionTaking<bool>('a');
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'char' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+
+  functionTaking<bool>("");
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'const char *' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+  functionTaking<bool>("abc");
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'const char *' -> bool
+  // CHECK-FIXES: functionTaking<bool>(true);
+
+  functionTaking<bool>(NULL);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'long' -> bool
+  // CHECK-FIXES: functionTaking<bool>(false);
+}
+
+void implicitCastToBoolFromUnaryMinusAndZeroLiterals() {
+  functionTaking<bool>(-0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: functionTaking<bool>((-0) != 0);
+
+  functionTaking<bool>(-0.0f);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool
+  // CHECK-FIXES: functionTaking<bool>((-0.0f) != 0.0f);
+
+  functionTaking<bool>(-0.0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'double' -> bool
+  // CHECK-FIXES: functionTaking<bool>((-0.0) != 0.0);
+}
+
+void implicitCastToBoolInWithOverloadedOperators() {
+  struct UserStruct {
+    int operator()(int x) { return x; }
+    int operator+(int y) { return y; }
+  };
+
+  UserStruct s;
+
+  functionTaking<bool>(s(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: functionTaking<bool>(s(0) != 0);
+
+  functionTaking<bool>(s + 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool
+  // CHECK-FIXES: functionTaking<bool>((s + 2) != 0);
+}
+
+int functionReturningInt();
+int* functionReturningPointer();
+
+void ignoreImplicitCastToBoolWhenDeclaringVariableInControlStatements() {
+  if (int integer = functionReturningInt()) {}
+
+  while (int* pointer = functionReturningPointer()) {}
+}
+
+void ignoreExplicitCastsToBool() {
+  int integer = 10;
+  bool boolComingFromInt = static_cast<bool>(integer);
+
+  float floating = 10.0f;
+  bool boolComingFromFloat = static_cast<bool>(floating);
+
+  char character = 'a';
+  bool boolComingFromChar = static_cast<bool>(character);
+
+  int* pointer = nullptr;
+  bool booleanComingFromPointer = static_cast<bool>(pointer);
+}
+
+void ignoreImplicitCastToBoolInMacroExpansions() {
+  int integer = 3;
+
+  #define CAST_TO_BOOL_IN_MACRO_BODY integer && false
+  bool boolFromMacroBody = CAST_TO_BOOL_IN_MACRO_BODY;
+
+  #define CAST_TO_BOOL_IN_MACRO_ARGUMENT(x) x || true
+  bool boolFromMacroArgument = CAST_TO_BOOL_IN_MACRO_ARGUMENT(integer);
+}
+
+namespace ignoreImplicitCastToBoolInTemplateInstantiations {
+
+template<typename T>
+void templateFunction() {
+  T unknownType = 0;
+  bool boolean = unknownType;
+}
+
+void useOfTemplateFunction() {
+  templateFunction<int>();
+}
+
+} // namespace ignoreImplicitCastToBoolInTemplateInstantiations
+
+namespace ignoreUserDefinedConversionOperator {
+
+struct StructWithUserConversion {
+  operator bool();
+};
+
+void useOfUserConversion() {
+  StructWithUserConversion structure;
+  functionTaking<bool>(structure);
+}
+
+} // namespace ignoreUserDefinedConversionOperator