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 ); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
