=?utf-8?q?Björn?= Svensson <bjorn.a.svens...@est.tech>,
=?utf-8?q?Björn?= Svensson <bjorn.a.svens...@est.tech>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/92...@github.com>
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Björn Svensson (bjosv)
<details>
<summary>Changes</summary>
`readability-implicit-bool-conversion` supports language-versions with
`LangOpts.Bool` which includes C23.
This PR corrects an issue that the fixer suggests `static_cast<>()` which
is not available in C23,
and will instead suggest C-style casts on other than C++.
The fixer will also suggest using `nullptr` instead of `0` and avoid a problem
with recursive fixes on C23.
The recursive issue, a function taking bool and a comparison, is now excluded
as in C++.
Before change:
```
functionTakingBool(integer1 == integer2);
^
( ) != 0
functionTakingBool((integer1 == integer2) != 0);
^
( ) != 0
```
```
AST snippet in C23 for functionTakingBool(integer1 == integer2):
`-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>
...
compared to AST in C++:
`-CallExpr 0x5815dec42a78 <line:21:3, col:42> 'void'
`-BinaryOperator 0x5815dec429f0 <col:22, col:34> 'bool' '=='
|-ImplicitCastExpr 0x5815dec429c0 <col:22> 'int'
<LValueToRValue>
...
```
---
Full diff: https://github.com/llvm/llvm-project/pull/92241.diff
4 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
(+15-1)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
- (modified)
clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
(+2-2)
- (added)
clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
(+354)
``````````diff
diff --git
a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 74152c6034510..704972b604380 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");
@@ -165,6 +167,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);
@@ -267,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(
@@ -281,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/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..a8c69858f76b6
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
@@ -0,0 +1,354 @@
+// 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 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;
+
+ 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 != nullptr);
+}
+
+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 != nullptr);
+}
+
+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 );
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/92241
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
