[clang-tools-extra] 729403e - [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (#92241)
via cfe-commits
cfe-commits at lists.llvm.org
Thu May 23 09:57:25 PDT 2024
Author: Björn Svensson
Date: 2024-05-23T18:57:21+02:00
New Revision: 729403e244c9970efa7aa17be32c197eb8e28fac
URL: https://github.com/llvm/llvm-project/commit/729403e244c9970efa7aa17be32c197eb8e28fac
DIFF: https://github.com/llvm/llvm-project/commit/729403e244c9970efa7aa17be32c197eb8e28fac.diff
LOG: [clang-tidy] Correcting issues in `readability-implicit-bool-conversion` on C23 (#92241)
`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++.
Added:
clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
Modified:
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 74152c6034510..28f5eada6d825 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()),
+ hasSourceExpression(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 741abc0a199a7..3e3195f6f6813 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -381,7 +381,9 @@ 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 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 1ea67a0b55e96..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`` 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);``,
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 );
+}
More information about the cfe-commits
mailing list