[clang-tools-extra] [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (PR #92241)
Björn Svensson via cfe-commits
cfe-commits at lists.llvm.org
Wed May 22 04:19:10 PDT 2024
https://github.com/bjosv updated https://github.com/llvm/llvm-project/pull/92241
>From 44ae41f00064dc477db0eb00b45fceff811cadec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Mon, 29 Apr 2024 12:49:59 +0200
Subject: [PATCH 1/5] [clang-tidy] Use C-style casts on C23 in
readability-implicit-bool-conversion
readability-implicit-bool-conversion supports language-versions which has
LangOpts.Bool and that includes C23. The problem is that the fixer suggests
`static_cast<>()` which is not available in C23.
Update checker to provide C-style casts when running on other than C++.
---
.../ImplicitBoolConversionCheck.cpp | 6 +
clang-tools-extra/docs/ReleaseNotes.rst | 3 +-
.../readability/implicit-bool-conversion.rst | 4 +-
.../readability/implicit-bool-conversion.c | 333 ++++++++++++++++++
4 files changed, 343 insertions(+), 3 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 74152c6034510..c2d1bc691d58a 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -165,6 +165,12 @@ bool needsSpacePrefix(SourceLocation Loc, ASTContext &Context) {
void fixGenericExprCastFromBool(DiagnosticBuilder &Diag,
const ImplicitCastExpr *Cast,
ASTContext &Context, StringRef OtherType) {
+ if (!Context.getLangOpts().CPlusPlus) {
+ Diag << FixItHint::CreateInsertion(Cast->getBeginLoc(),
+ (Twine("(") + OtherType + ")").str());
+ return;
+ }
+
const Expr *SubExpr = Cast->getSubExpr();
const bool NeedParens = !isa<ParenExpr>(SubExpr->IgnoreImplicit());
const bool NeedSpace = needsSpacePrefix(Cast->getBeginLoc(), Context);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 19f8307412956..d19f83793a6c4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -368,7 +368,8 @@ Changes in existing checks
- Improved :doc:`readability-implicit-bool-conversion
<clang-tidy/checks/readability/implicit-bool-conversion>` check to provide
valid fix suggestions for ``static_cast`` without a preceding space and
- fixed problem with duplicate parentheses in double implicit casts.
+ fixed problem with duplicate parentheses in double implicit casts. Corrected
+ the fix suggestions for C23 by using C-style casts instead of ``static_cast``.
- Improved :doc:`readability-redundant-inline-specifier
<clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
index 1ea67a0b55e96..643af488ae3df 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
@@ -96,8 +96,8 @@ The rules for generating fix-it hints are:
- ``if (!pointer)`` is changed to ``if (pointer == nullptr)``,
- in case of conversions from bool to other built-in types, an explicit
- ``static_cast`` is proposed to make it clear that a conversion is taking
- place:
+ ``static_cast`` (or a C-style cast for C23) is proposed to make it clear that
+ a conversion is taking place:
- ``int integer = boolean;`` is changed to
``int integer = static_cast<int>(boolean);``,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
new file mode 100644
index 0000000000000..c264372efaf25
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
@@ -0,0 +1,333 @@
+// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t -- -- -std=c23
+
+#undef NULL
+#define NULL 0L
+
+void functionTakingBool(bool);
+void functionTakingInt(int);
+void functionTakingUnsignedLong(unsigned long);
+void functionTakingChar(char);
+void functionTakingFloat(float);
+void functionTakingDouble(double);
+void functionTakingSignedChar(signed char);
+
+
+////////// Implicit conversion from bool.
+
+void implicitConversionFromBoolSimpleCases() {
+ bool boolean = true;
+
+ functionTakingBool(boolean);
+
+ functionTakingInt(boolean);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion]
+ // CHECK-FIXES: functionTakingInt((int)boolean);
+
+ functionTakingUnsignedLong(boolean);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit conversion 'bool' -> 'unsigned long'
+ // CHECK-FIXES: functionTakingUnsignedLong((unsigned long)boolean);
+
+ functionTakingChar(boolean);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'bool' -> 'char'
+ // CHECK-FIXES: functionTakingChar((char)boolean);
+
+ functionTakingFloat(boolean);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'bool' -> 'float'
+ // CHECK-FIXES: functionTakingFloat((float)boolean);
+
+ functionTakingDouble(boolean);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'double'
+ // CHECK-FIXES: functionTakingDouble((double)boolean);
+}
+
+float implicitConversionFromBoolInReturnValue() {
+ bool boolean = false;
+ return boolean;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'bool' -> 'float'
+ // CHECK-FIXES: return (float)boolean;
+}
+
+void implicitConversionFromBoolInSingleBoolExpressions(bool b1, bool b2) {
+ bool boolean = true;
+ boolean = b1 ^ b2;
+ boolean |= !b1 || !b2;
+ boolean &= b1;
+
+ int integer = boolean - 3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: int integer = (int)boolean - 3;
+
+ float floating = boolean / 0.3f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion 'bool' -> 'float'
+ // CHECK-FIXES: float floating = (float)boolean / 0.3f;
+
+ char character = boolean;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion 'bool' -> 'char'
+ // CHECK-FIXES: char character = (char)boolean;
+}
+
+void implicitConversionFromBoolInComplexBoolExpressions() {
+ bool boolean = true;
+ bool anotherBoolean = false;
+
+ int integer = boolean && anotherBoolean;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: int integer = (int)boolean && (int)anotherBoolean;
+
+ float floating = (boolean || anotherBoolean) * 0.3f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: float floating = ((int)boolean || (int)anotherBoolean) * 0.3f;
+
+ double doubleFloating = (boolean && (anotherBoolean || boolean)) * 0.3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-MESSAGES: :[[@LINE-2]]:40: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-MESSAGES: :[[@LINE-3]]:58: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: double doubleFloating = ((int)boolean && ((int)anotherBoolean || (int)boolean)) * 0.3;
+}
+
+void implicitConversionFromBoolLiterals() {
+ functionTakingInt(true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: functionTakingInt(1);
+
+ functionTakingUnsignedLong(false);
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit conversion 'bool' -> 'unsigned long'
+ // CHECK-FIXES: functionTakingUnsignedLong(0u);
+
+ functionTakingSignedChar(true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit conversion 'bool' -> 'signed char'
+ // CHECK-FIXES: functionTakingSignedChar(1);
+
+ functionTakingFloat(false);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'bool' -> 'float'
+ // CHECK-FIXES: functionTakingFloat(0.0f);
+
+ functionTakingDouble(true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'double'
+ // CHECK-FIXES: functionTakingDouble(1.0);
+}
+
+void ignoreExplicitCastsFromBool() {
+ bool boolean = true;
+
+ int integer = (int)boolean + 3;
+ float floating = (float)boolean * 0.3f;
+ char character = (char)boolean;
+}
+
+void ignoreImplicitConversionFromBoolInMacroExpansions() {
+ 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);
+}
+
+////////// Implicit conversions to bool.
+
+void implicitConversionToBoolSimpleCases() {
+ int integer = 10;
+ functionTakingBool(integer);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(integer != 0);
+
+ unsigned long unsignedLong = 10;
+ functionTakingBool(unsignedLong);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'unsigned long' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(unsignedLong != 0u);
+
+ float floating = 0.0f;
+ functionTakingBool(floating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'float' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(floating != 0.0f);
+
+ double doubleFloating = 1.0f;
+ functionTakingBool(doubleFloating);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'double' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(doubleFloating != 0.0);
+
+ signed char character = 'a';
+ functionTakingBool(character);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'signed char' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(character != 0);
+
+ int* pointer = nullptr;
+ functionTakingBool(pointer);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int *' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(pointer != 0);
+}
+
+void implicitConversionToBoolInSingleExpressions() {
+ int integer = 10;
+ bool boolComingFromInt;
+ boolComingFromInt = integer;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: boolComingFromInt = (integer != 0);
+
+ float floating = 10.0f;
+ bool boolComingFromFloat;
+ boolComingFromFloat = floating;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'float' -> 'bool'
+ // CHECK-FIXES: boolComingFromFloat = (floating != 0.0f);
+
+ signed char character = 'a';
+ bool boolComingFromChar;
+ boolComingFromChar = character;
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'signed char' -> 'bool'
+ // CHECK-FIXES: boolComingFromChar = (character != 0);
+
+ int* pointer = nullptr;
+ bool boolComingFromPointer;
+ boolComingFromPointer = pointer;
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit conversion 'int *' -> 'bool'
+ // CHECK-FIXES: boolComingFromPointer = (pointer != 0);
+}
+
+void implicitConversionToBoolInComplexExpressions() {
+ bool boolean = true;
+
+ int integer = 10;
+ int anotherInteger = 20;
+ bool boolComingFromInteger;
+ boolComingFromInteger = integer + anotherInteger;
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: boolComingFromInteger = ((integer + anotherInteger) != 0);
+}
+
+void implicitConversionInNegationExpressions() {
+ int integer = 10;
+ bool boolComingFromNegatedInt;
+ boolComingFromNegatedInt = !integer;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: boolComingFromNegatedInt = ((!integer) != 0);
+}
+
+bool implicitConversionToBoolInReturnValue() {
+ float floating = 1.0f;
+ return floating;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'float' -> 'bool'
+ // CHECK-FIXES: return floating != 0.0f;
+}
+
+void implicitConversionToBoolFromLiterals() {
+ functionTakingBool(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(false);
+
+ functionTakingBool(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool(2ul);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'unsigned long' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool(0.0f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'float' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(false);
+
+ functionTakingBool(1.0f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'float' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool(2.0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'double' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool('\0');
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(false);
+
+ functionTakingBool('a');
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool("");
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'char *' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool("abc");
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'char *' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(true);
+
+ functionTakingBool(NULL);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'long' -> 'bool'
+ // CHECK-FIXES: functionTakingBool(false);
+}
+
+void implicitConversionToBoolFromUnaryMinusAndZeroLiterals() {
+ functionTakingBool(-0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: functionTakingBool((-0) != 0);
+
+ functionTakingBool(-0.0f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'float' -> 'bool'
+ // CHECK-FIXES: functionTakingBool((-0.0f) != 0.0f);
+
+ functionTakingBool(-0.0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'double' -> 'bool'
+ // CHECK-FIXES: functionTakingBool((-0.0) != 0.0);
+}
+
+void ignoreExplicitCastsToBool() {
+ int integer = 10;
+ bool boolComingFromInt = (bool)integer;
+
+ float floating = 10.0f;
+ bool boolComingFromFloat = (bool)floating;
+
+ char character = 'a';
+ bool boolComingFromChar = (bool)character;
+
+ int* pointer = nullptr;
+ bool booleanComingFromPointer = (bool)pointer;
+}
+
+void ignoreImplicitConversionToBoolInMacroExpansions() {
+ 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);
+}
+
+int implicitConversionReturnInt()
+{
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: return 1
+}
+
+int implicitConversionReturnIntWithParens()
+{
+ return (true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: return 1
+}
+
+bool implicitConversionReturnBool()
+{
+ return 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: return true
+}
+
+bool implicitConversionReturnBoolWithParens()
+{
+ return (1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool'
+ // CHECK-FIXES: return true
+}
+
+int keepCompactReturnInC_PR71848() {
+ bool foo = false;
+ return( foo );
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion]
+// CHECK-FIXES: return(int)( foo );
+}
>From b8458b8ddc473a11f128d2a66e37b9572d5799e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Mon, 29 Apr 2024 12:49:59 +0200
Subject: [PATCH 2/5] [clang-tidy] Use nullptr in ImplicitBoolConversionCheck
on C23
---
.../clang-tidy/readability/ImplicitBoolConversionCheck.cpp | 4 +++-
.../checkers/readability/implicit-bool-conversion.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index c2d1bc691d58a..ee86336b4b95b 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -50,7 +50,9 @@ StringRef getZeroLiteralToCompareWithForType(CastKind CastExprKind,
case CK_PointerToBoolean:
case CK_MemberPointerToBoolean: // Fall-through on purpose.
- return Context.getLangOpts().CPlusPlus11 ? "nullptr" : "0";
+ return (Context.getLangOpts().CPlusPlus11 || Context.getLangOpts().C23)
+ ? "nullptr"
+ : "0";
default:
llvm_unreachable("Unexpected cast kind");
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
index c264372efaf25..496f85b7d797d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
@@ -158,7 +158,7 @@ void implicitConversionToBoolSimpleCases() {
int* pointer = nullptr;
functionTakingBool(pointer);
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'int *' -> 'bool'
- // CHECK-FIXES: functionTakingBool(pointer != 0);
+ // CHECK-FIXES: functionTakingBool(pointer != nullptr);
}
void implicitConversionToBoolInSingleExpressions() {
@@ -184,7 +184,7 @@ void implicitConversionToBoolInSingleExpressions() {
bool boolComingFromPointer;
boolComingFromPointer = pointer;
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit conversion 'int *' -> 'bool'
- // CHECK-FIXES: boolComingFromPointer = (pointer != 0);
+ // CHECK-FIXES: boolComingFromPointer = (pointer != nullptr);
}
void implicitConversionToBoolInComplexExpressions() {
>From d0a994fb403c40d95c551be5de2343beb661f330 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Wed, 15 May 2024 08:41:18 +0200
Subject: [PATCH 3/5] [clang-tidy] Avoid recursive fixes in
ImplicitBoolConversionCheck on C23
Exclude a common implicit casts case where C23 has an additional cast
compared to C++ which results in recursive fixes.
Before change:
functionTakingBool(integer1 == integer2);
^
( ) != 0
functionTakingBool((integer1 == integer2) != 0);
^
( ) != 0
AST in C23:
`-CallExpr 0x5bf18d403b58 <line:21:3, col:42> 'void'
`-ImplicitCastExpr 0x5bf18d403b80 <col:22, col:34> 'bool' <IntegralToBoolean>
`-BinaryOperator 0x5bf18d403ae8 <col:22, col:34> 'int' '=='
|-ImplicitCastExpr 0x5bf18d403ab8 <col:22> 'int' <LValueToRValue>
...
AST in C++:
`-CallExpr 0x5815dec42a78 <line:21:3, col:42> 'void'
`-BinaryOperator 0x5815dec429f0 <col:22, col:34> 'bool' '=='
|-ImplicitCastExpr 0x5815dec429c0 <col:22> 'int' <LValueToRValue>
...
---
.../ImplicitBoolConversionCheck.cpp | 6 ++++++
.../readability/implicit-bool-conversion.c | 21 +++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index ee86336b4b95b..704972b604380 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -275,6 +275,10 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
auto BoolXor =
binaryOperator(hasOperatorName("^"), hasLHS(ImplicitCastFromBool),
hasRHS(ImplicitCastFromBool));
+ auto ComparisonInCall =
+ allOf(hasParent(callExpr()),
+ has(binaryOperator(hasAnyOperatorName("==", "!="))));
+
Finder->addMatcher(
traverse(TK_AsIs,
implicitCastExpr(
@@ -289,6 +293,8 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
stmt(anyOf(ifStmt(), whileStmt()), has(declStmt())))),
// Exclude cases common to implicit cast to and from bool.
unless(ExceptionCases), unless(has(BoolXor)),
+ // Exclude C23 cases common to implicit cast to bool.
+ unless(ComparisonInCall),
// Retrieve also parent statement, to check if we need
// additional parens in replacement.
optionally(hasParent(stmt().bind("parentStmt"))),
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
index 496f85b7d797d..a8c69858f76b6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
@@ -109,6 +109,27 @@ void implicitConversionFromBoolLiterals() {
// CHECK-FIXES: functionTakingDouble(1.0);
}
+void implicitConversionFromBoolInComparisons() {
+ bool boolean = true;
+ int integer = 0;
+
+ functionTakingBool(boolean == integer);
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: functionTakingBool((int)boolean == integer);
+
+ functionTakingBool(integer != boolean);
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'bool' -> 'int'
+ // CHECK-FIXES: functionTakingBool(integer != (int)boolean);
+}
+
+void ignoreBoolComparisons() {
+ bool boolean = true;
+ bool anotherBoolean = false;
+
+ functionTakingBool(boolean == anotherBoolean);
+ functionTakingBool(boolean != anotherBoolean);
+}
+
void ignoreExplicitCastsFromBool() {
bool boolean = true;
>From ccb42bbe314df6950ebd0609bd282b7bfec1238c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Thu, 16 May 2024 23:12:53 +0200
Subject: [PATCH 4/5] fixup: improved wording in documentation due to review
comments
---
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++-
.../checks/readability/implicit-bool-conversion.rst | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d19f83793a6c4..442e18aafe402 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -369,7 +369,8 @@ Changes in existing checks
<clang-tidy/checks/readability/implicit-bool-conversion>` check to provide
valid fix suggestions for ``static_cast`` without a preceding space and
fixed problem with duplicate parentheses in double implicit casts. Corrected
- the fix suggestions for C23 by using C-style casts instead of ``static_cast``.
+ the fix suggestions for C23 and later by using C-style casts instead of
+ ``static_cast``.
- Improved :doc:`readability-redundant-inline-specifier
<clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
index 643af488ae3df..1ab21ffeb4228 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
@@ -96,8 +96,8 @@ The rules for generating fix-it hints are:
- ``if (!pointer)`` is changed to ``if (pointer == nullptr)``,
- in case of conversions from bool to other built-in types, an explicit
- ``static_cast`` (or a C-style cast for C23) is proposed to make it clear that
- a conversion is taking place:
+ ``static_cast`` (or a C-style cast since C23) is proposed to make it clear
+ that a conversion is taking place:
- ``int integer = boolean;`` is changed to
``int integer = static_cast<int>(boolean);``,
>From 5506c4c5f5564a752eac3828a6d1f461320423fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson at est.tech>
Date: Wed, 22 May 2024 13:09:40 +0200
Subject: [PATCH 5/5] fixup: use hasSourceExpression() instead of has()
---
.../clang-tidy/readability/ImplicitBoolConversionCheck.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 704972b604380..28f5eada6d825 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -275,9 +275,9 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
auto BoolXor =
binaryOperator(hasOperatorName("^"), hasLHS(ImplicitCastFromBool),
hasRHS(ImplicitCastFromBool));
- auto ComparisonInCall =
- allOf(hasParent(callExpr()),
- has(binaryOperator(hasAnyOperatorName("==", "!="))));
+ auto ComparisonInCall = allOf(
+ hasParent(callExpr()),
+ hasSourceExpression(binaryOperator(hasAnyOperatorName("==", "!="))));
Finder->addMatcher(
traverse(TK_AsIs,
More information about the cfe-commits
mailing list