LegalizeAdulthood created this revision.
LegalizeAdulthood added a reviewer: alexfh.
LegalizeAdulthood added a subscriber: cfe-commits.
Expand the simplify boolean expression check to handle implicit conversion of
integral types to bool
and improve the handling of implicit conversion of member pointers to bool.
Implicit conversion of member pointers are replaced with explicit comparisons
to `nullptr`.
Implicit conversions of integral types are replaced with explicit comparisons
to `0`.
http://reviews.llvm.org/D16308
Files:
clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tidy/readability/SimplifyBooleanExprCheck.h
docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
test/clang-tidy/readability-simplify-bool-expr.cpp
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<bool>(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<bool>(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<bool>(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<bool>(~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<bool>(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;{{$}}
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<bool>(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<bool>(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.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<bool>(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<bool>(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<ImplicitCastExpr>(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<ImplicitCastExpr>(E))
+ return ImpCast->getCastKind() == CK_IntegralToBoolean;
return false;
}
@@ -182,13 +190,19 @@
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());
}
}
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,19 +230,28 @@
if (needsNullPtrComparison(E))
return (getText(Result, *E) + " == nullptr").str();
+ if (needsZeroComparison(E))
+ return (getText(Result, *E) + " == 0").str();
+
return ("!" + asBool(Text, NeedsStaticCast));
}
if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
