Index: clang-tidy/readability/SimplifyBooleanExprCheck.h =================================================================== --- clang-tidy/readability/SimplifyBooleanExprCheck.h +++ clang-tidy/readability/SimplifyBooleanExprCheck.h @@ -52,6 +52,8 @@ /// 5. Implicit casts to `bool` are replaced with explicit casts to `bool`. /// 6. Object expressions with `explicit operator bool` conversion operators /// are replaced with explicit casts to `bool`. +/// 7. Implicit conversions of integral types to `bool` are replaced with +/// explicit comparisons to `0`. /// /// Examples: /// 1. The ternary assignment `bool b = (i < 0) ? true : false;` has redundant @@ -72,11 +74,11 @@ /// /// The ternary assignment `bool b = (i & 1) ? true : false;` has an /// implicit conversion of `i & 1` to `bool` and becomes -/// `bool b = static_cast(i & 1);`. +/// `bool b = i & 1 != 0;`. /// /// 5. The conditional return `if (i & 1) return true; else return false;` has /// an implicit conversion of an integer quantity `i & 1` to `bool` and -/// becomes `return static_cast(i & 1);` +/// becomes `return i & 1 != 0;` /// /// 6. Given `struct X { explicit operator bool(); };`, and an instance `x` of /// `struct X`, the conditional return `if (x) return true; return false;` Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp =================================================================== --- clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -148,7 +148,15 @@ bool needsNullPtrComparison(const Expr *E) { if (const auto *ImpCast = dyn_cast(E)) - return ImpCast->getCastKind() == CK_PointerToBoolean; + return ImpCast->getCastKind() == CK_PointerToBoolean + || ImpCast->getCastKind() == CK_MemberPointerToBoolean; + + return false; +} + +bool needsZeroComparison(const Expr *E) { + if (const auto *ImpCast = dyn_cast(E)) + return ImpCast->getCastKind() == CK_IntegralToBoolean; return false; } @@ -182,6 +190,9 @@ if (needsNullPtrComparison(UnOp->getSubExpr())) return (getText(Result, *UnOp->getSubExpr()) + " != nullptr").str(); + if (needsZeroComparison(UnOp->getSubExpr())) + return (getText(Result, *UnOp->getSubExpr()) + " != 0").str(); + return replacementExpression(Result, false, UnOp->getSubExpr()); } } @@ -189,6 +200,9 @@ if (needsNullPtrComparison(E)) return (getText(Result, *E) + " == nullptr").str(); + if (needsZeroComparison(E)) + return (getText(Result, *E) + " == 0").str(); + StringRef NegatedOperator; const Expr *LHS = nullptr; const Expr *RHS = nullptr; @@ -216,6 +230,9 @@ if (needsNullPtrComparison(E)) return (getText(Result, *E) + " == nullptr").str(); + if (needsZeroComparison(E)) + return (getText(Result, *E) + " == 0").str(); + return ("!" + asBool(Text, NeedsStaticCast)); } @@ -223,12 +240,18 @@ if (UnOp->getOpcode() == UO_LNot) { if (needsNullPtrComparison(UnOp->getSubExpr())) return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str(); + + if (needsZeroComparison(UnOp->getSubExpr())) + return (getText(Result, *UnOp->getSubExpr()) + " == 0").str(); } } if (needsNullPtrComparison(E)) return (getText(Result, *E) + " != nullptr").str(); + if (needsZeroComparison(E)) + return (getText(Result, *E) + " != 0").str(); + return asBool(getText(Result, *E), NeedsStaticCast); } @@ -582,7 +605,7 @@ // The body shouldn't be empty because the matcher ensures that it must // contain at least two statements: // 1) A `return` statement returning a boolean literal `false` or `true` - // 2) An `if` statement with no `else` clause that consists fo a single + // 2) An `if` statement with no `else` clause that consists of a single // `return` statement returning the opposite boolean literal `true` or // `false`. assert(Compound->size() >= 2); Index: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst =================================================================== --- docs/clang-tidy/checks/readability-simplify-boolean-expr.rst +++ docs/clang-tidy/checks/readability-simplify-boolean-expr.rst @@ -40,6 +40,8 @@ 5. Implicit casts to ``bool`` are replaced with explicit casts to ``bool``. 6. Object expressions with ``explicit operator bool`` conversion operators are replaced with explicit casts to ``bool``. + 7. Implicit conversions of integral types to ``bool`` are replaced with + explicit comparisons to ``0``. Examples: 1. The ternary assignment ``bool b = (i < 0) ? true : false;`` has redundant @@ -60,11 +62,11 @@ The ternary assignment ``bool b = (i & 1) ? true : false;`` has an implicit conversion of ``i & 1`` to ``bool`` and becomes - ``bool b = static_cast(i & 1);``. + ``bool b = i & 1 != 0;``. 5. The conditional return ``if (i & 1) return true; else return false;`` has an implicit conversion of an integer quantity ``i & 1`` to ``bool`` and - becomes ``return static_cast(i & 1);`` + becomes ``return i & 1 != 0;`` 6. Given ``struct X { explicit operator bool(); };``, and an instance ``x`` of ``struct X``, the conditional return ``if (x) return true; return false;`` Index: test/clang-tidy/readability-simplify-bool-expr.cpp =================================================================== --- test/clang-tidy/readability-simplify-bool-expr.cpp +++ test/clang-tidy/readability-simplify-bool-expr.cpp @@ -690,7 +690,7 @@ } } // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return -// CHECK-FIXES: {{^}} return static_cast(i & 1);{{$}} +// CHECK-FIXES: {{^}} return i & 1 != 0;{{$}} bool negated_if_implicit_bool_expr(int i) { if (i - 1) { @@ -700,7 +700,7 @@ } } // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return -// CHECK-FIXES: {{^}} return !static_cast(i - 1);{{$}} +// CHECK-FIXES: {{^}} return i - 1 == 0;{{$}} bool implicit_int(int i) { if (i) { @@ -710,7 +710,7 @@ } } // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return -// CHECK-FIXES: {{^}} return static_cast(i);{{$}} +// CHECK-FIXES: {{^}} return i != 0;{{$}} bool explicit_bool(bool b) { if (b) { @@ -757,7 +757,7 @@ } } // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return -// CHECK-FIXES: {{^}} return static_cast(~i);{{$}} +// CHECK-FIXES: {{^}} return ~i != 0;{{$}} bool logical_or(bool a, bool b) { if (a || b) { @@ -830,7 +830,7 @@ bool b = i ? true : false; } // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{.*}} in ternary expression result -// CHECK-FIXES: bool b = static_cast(i);{{$}} +// CHECK-FIXES: bool b = i != 0;{{$}} bool non_null_pointer_condition(int *p1) { if (p1) { @@ -895,3 +895,28 @@ // CHECK-MESSAGES: :[[@LINE-6]]:12: warning: {{.*}} in conditional return // CHECK-FIXES: {{^}} if (b) { // CHECK-FIXES: {{^}}#define SOMETHING_WICKED false + +bool integer_not_zero(int i) { + if (i) { + return false; + } else { + return true; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return i == 0;{{$}} + +class A { +public: + int m; +}; + +bool member_pointer_nullptr(int A::*p) { + if (p) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: return p != nullptr;{{$}}