Author: 4m4n-x-B4w4ne Date: 2024-12-28T15:35:30+08:00 New Revision: 5bec2b71b44ddff44aa4d8534b58a5561389bb1d
URL: https://github.com/llvm/llvm-project/commit/5bec2b71b44ddff44aa4d8534b58a5561389bb1d DIFF: https://github.com/llvm/llvm-project/commit/5bec2b71b44ddff44aa4d8534b58a5561389bb1d.diff LOG: Added options to readability-implicit-bool-conversion (#120087) As given in the issue #36323 , I added two new options in the clang-tools-extra/clan-tidy/readibility/ImplicitBoolConversionCheck.cpp and header file. I have also written new test cases to test these new options in test/readibility directory. Added: clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-check.cpp Modified: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h 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 f9fd1d903e231e..48851da143068f 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -259,13 +259,17 @@ ImplicitBoolConversionCheck::ImplicitBoolConversionCheck( AllowIntegerConditions(Options.get("AllowIntegerConditions", false)), AllowPointerConditions(Options.get("AllowPointerConditions", false)), UseUpperCaseLiteralSuffix( - Options.get("UseUpperCaseLiteralSuffix", false)) {} + Options.get("UseUpperCaseLiteralSuffix", false)), + CheckConversionsToBool(Options.get("CheckConversionsToBool", true)), + CheckConversionsFromBool(Options.get("CheckConversionsFromBool", true)) {} void ImplicitBoolConversionCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "AllowIntegerConditions", AllowIntegerConditions); Options.store(Opts, "AllowPointerConditions", AllowPointerConditions); Options.store(Opts, "UseUpperCaseLiteralSuffix", UseUpperCaseLiteralSuffix); + Options.store(Opts, "CheckConversionsToBool", CheckConversionsToBool); + Options.store(Opts, "CheckConversionsFromBool", CheckConversionsFromBool); } void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) { @@ -277,6 +281,7 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) { expr(hasType(qualType().bind("type")), hasParent(initListExpr(hasParent(explicitCastExpr( hasType(qualType(equalsBoundNode("type")))))))))); + auto ImplicitCastFromBool = implicitCastExpr( anyOf(hasCastKind(CK_IntegralCast), hasCastKind(CK_IntegralToFloating), // Prior to C++11 cast from bool literal to pointer was allowed. @@ -287,72 +292,84 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) { auto BoolXor = binaryOperator(hasOperatorName("^"), hasLHS(ImplicitCastFromBool), hasRHS(ImplicitCastFromBool)); - auto ComparisonInCall = allOf( - hasParent(callExpr()), - hasSourceExpression(binaryOperator(hasAnyOperatorName("==", "!=")))); - auto IsInCompilerGeneratedFunction = hasAncestor(namedDecl(anyOf( isImplicit(), functionDecl(isDefaulted()), functionTemplateDecl()))); - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr( - anyOf(hasCastKind(CK_IntegralToBoolean), - hasCastKind(CK_FloatingToBoolean), - hasCastKind(CK_PointerToBoolean), - hasCastKind(CK_MemberPointerToBoolean)), - // Exclude cases of C23 comparison result. - unless(allOf(isC23(), - hasSourceExpression(ignoringParens( - binaryOperator(hasAnyOperatorName( - ">", ">=", "==", "!=", "<", "<=")))))), - // Exclude case of using if or while statements with variable - // declaration, e.g.: - // if (int var = functionCall()) {} - unless(hasParent( - 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"))), - unless(isInTemplateInstantiation()), - unless(IsInCompilerGeneratedFunction)) - .bind("implicitCastToBool")), - this); - - auto BoolComparison = binaryOperator(hasAnyOperatorName("==", "!="), - hasLHS(ImplicitCastFromBool), - hasRHS(ImplicitCastFromBool)); - auto BoolOpAssignment = binaryOperator(hasAnyOperatorName("|=", "&="), - hasLHS(expr(hasType(booleanType())))); - auto BitfieldAssignment = binaryOperator( - hasLHS(memberExpr(hasDeclaration(fieldDecl(hasBitWidth(1)))))); - auto BitfieldConstruct = cxxConstructorDecl(hasDescendant(cxxCtorInitializer( - withInitializer(equalsBoundNode("implicitCastFromBool")), - forField(hasBitWidth(1))))); - Finder->addMatcher( - traverse( - TK_AsIs, - implicitCastExpr( - ImplicitCastFromBool, unless(ExceptionCases), - // Exclude comparisons of bools, as they are always cast to - // integers in such context: - // bool_expr_a == bool_expr_b - // bool_expr_a != bool_expr_b - unless(hasParent( - binaryOperator(anyOf(BoolComparison, BoolXor, - BoolOpAssignment, BitfieldAssignment)))), - implicitCastExpr().bind("implicitCastFromBool"), - unless(hasParent(BitfieldConstruct)), - // Check also for nested casts, for example: bool -> int -> float. - anyOf(hasParent(implicitCastExpr().bind("furtherImplicitCast")), - anything()), - unless(isInTemplateInstantiation()), - unless(IsInCompilerGeneratedFunction))), - this); + if (CheckConversionsToBool) { + auto ComparisonInCall = allOf( + hasParent(callExpr()), + hasSourceExpression(binaryOperator(hasAnyOperatorName("==", "!=")))); + + Finder->addMatcher( + traverse( + TK_AsIs, + implicitCastExpr( + anyOf(hasCastKind(CK_IntegralToBoolean), + hasCastKind(CK_FloatingToBoolean), + hasCastKind(CK_PointerToBoolean), + hasCastKind(CK_MemberPointerToBoolean)), + // Exclude cases of C23 comparison result. + unless(allOf(isC23(), + hasSourceExpression(ignoringParens( + binaryOperator(hasAnyOperatorName( + ">", ">=", "==", "!=", "<", "<=")))))), + // Exclude case of using if or while statements with variable + // declaration, e.g.: + // if (int var = functionCall()) {} + unless(hasParent( + 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"))), + unless(isInTemplateInstantiation()), + unless(IsInCompilerGeneratedFunction)) + .bind("implicitCastToBool")), + this); + } + + if (CheckConversionsFromBool) { + + auto BoolComparison = binaryOperator(hasAnyOperatorName("==", "!="), + hasLHS(ImplicitCastFromBool), + hasRHS(ImplicitCastFromBool)); + + auto BoolOpAssignment = binaryOperator( + hasAnyOperatorName("|=", "&="), hasLHS(expr(hasType(booleanType())))); + + auto BitfieldAssignment = binaryOperator( + hasLHS(memberExpr(hasDeclaration(fieldDecl(hasBitWidth(1)))))); + + auto BitfieldConstruct = + cxxConstructorDecl(hasDescendant(cxxCtorInitializer( + withInitializer(equalsBoundNode("implicitCastFromBool")), + forField(hasBitWidth(1))))); + + Finder->addMatcher( + traverse( + TK_AsIs, + implicitCastExpr( + ImplicitCastFromBool, unless(ExceptionCases), + // Exclude comparisons of bools, as they are + // always cast to integers in such context: + // bool_expr_a == bool_expr_b + // bool_expr_a != bool_expr_b + unless(hasParent(binaryOperator(anyOf(BoolComparison, BoolXor, + BoolOpAssignment, + BitfieldAssignment)))), + implicitCastExpr().bind("implicitCastFromBool"), + unless(hasParent(BitfieldConstruct)), + // Check also for nested casts, for example: + // bool -> int -> float. + anyOf(hasParent(implicitCastExpr().bind("furtherImplicitCast")), + anything()), + unless(isInTemplateInstantiation()), + unless(IsInCompilerGeneratedFunction))), + this); + } } void ImplicitBoolConversionCheck::check( diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h index 5947f7316e67cc..b0c3c2943e649c 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h @@ -37,6 +37,8 @@ class ImplicitBoolConversionCheck : public ClangTidyCheck { const bool AllowIntegerConditions; const bool AllowPointerConditions; const bool UseUpperCaseLiteralSuffix; + const bool CheckConversionsToBool; + const bool CheckConversionsFromBool; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index fabd0cc78ac645..8c360222ce43da 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -342,10 +342,11 @@ Changes in existing checks diagnostic. - Improved :doc:`readability-implicit-bool-conversion - <clang-tidy/checks/readability/implicit-bool-conversion>` check - by adding the option `UseUpperCaseLiteralSuffix` to select the - case of the literal suffix in fixes and fixing false positive for implicit - conversion of comparison result in C23. + <clang-tidy/checks/readability/implicit-bool-conversion>` check by adding the + option `UseUpperCaseLiteralSuffix` to select the case of the literal suffix in + fixes and fixing false positive for implicit conversion of comparison result in + C23, and by adding the option `CheckConversionsToBool` or + `CheckConversionsFromBool` to configure checks for conversions involving ``bool``. - Improved :doc:`readability-redundant-smartptr-get <clang-tidy/checks/readability/redundant-smartptr-get>` check to 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 88cff387f4c165..f7c15ffa2da51b 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 @@ -147,3 +147,41 @@ Options if (foo) {} // ^ propose replacement default: if (foo != 0u) {} // ^ propose replacement with option `UseUpperCaseLiteralSuffix`: if (foo != 0U) {} + +.. option:: CheckConversionsToBool + + When `true`, the check diagnoses implicit conversions to ``bool``. + Default is `true`. + + Example + + .. code-block:: c++ + + int x = 42; + if (x) {} + // ^ propose replacement: if (x != 0) {} + + float f = 3.14; + if (f) {} + // ^ propose replacement: if (f != 0.0f) {} + +.. option:: CheckConversionsFromBool + + When `true`, the check diagnoses implicit conversions from ``bool``. + Default is `true`. + + Example + + .. code-block:: c++ + + bool b = true; + + int x = b; + // ^ propose replacement: int x = b ? 1 : 0; + + float f = b; + // ^ propose replacement: float f = b ? 1.0f : 0.0f; + + int* p = b; + // ^ propose replacement: int* p = b ? some_ptr : nullptr; + diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-check.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-check.cpp new file mode 100644 index 00000000000000..8ba4635de1704f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-check.cpp @@ -0,0 +1,57 @@ +// RUN: %check_clang_tidy -check-suffix=FROM %s readability-implicit-bool-conversion %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-implicit-bool-conversion.CheckConversionsToBool: false, \ +// RUN: readability-implicit-bool-conversion.CheckConversionsFromBool: true \ +// RUN: }}' -- -std=c23 +// RUN: %check_clang_tidy -check-suffix=TO %s readability-implicit-bool-conversion %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-implicit-bool-conversion.CheckConversionsToBool: true, \ +// RUN: readability-implicit-bool-conversion.CheckConversionsFromBool: false \ +// RUN: }}' -- -std=c23 +// RUN: %check_clang_tidy -check-suffix=NORMAL %s readability-implicit-bool-conversion %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-implicit-bool-conversion.CheckConversionsToBool: false, \ +// RUN: readability-implicit-bool-conversion.CheckConversionsFromBool: false \ +// RUN: }}' -- -std=c23 +// RUN: %check_clang_tidy -check-suffix=TO,FROM %s readability-implicit-bool-conversion %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-implicit-bool-conversion.CheckConversionsToBool: true, \ +// RUN: readability-implicit-bool-conversion.CheckConversionsFromBool: true \ +// RUN: }}' -- -std=c23 + +// Test various implicit bool conversions in diff erent contexts +void TestImplicitBoolConversion() { + // Basic type conversions to bool + int intValue = 42; + if (intValue) // CHECK-MESSAGES-TO: :[[@LINE]]:9: warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion] + // CHECK-FIXES-TO: if (intValue != 0) + (void)0; + + float floatValue = 3.14f; + while (floatValue) // CHECK-MESSAGES-TO: :[[@LINE]]:12: warning: implicit conversion 'float' -> 'bool' [readability-implicit-bool-conversion] + // CHECK-FIXES-TO: while (floatValue != 0.0f) + break; + + char charValue = 'a'; + do { + break; + } while (charValue); // CHECK-MESSAGES-TO: :[[@LINE]]:14: warning: implicit conversion 'char' -> 'bool' [readability-implicit-bool-conversion] + // CHECK-FIXES-TO: } while (charValue != 0); + + // Pointer conversions to bool + int* ptrValue = &intValue; + if (ptrValue) // CHECK-MESSAGES-TO: :[[@LINE]]:9: warning: implicit conversion 'int *' -> 'bool' [readability-implicit-bool-conversion] + // CHECK-FIXES-TO: if (ptrValue != nullptr) + (void)0; + + // Conversions from bool to other types + bool boolValue = true; + int intFromBool = boolValue; // CHECK-MESSAGES-FROM: :[[@LINE]]:23: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion] + // CHECK-FIXES-FROM: int intFromBool = static_cast<int>(boolValue); + + float floatFromBool = boolValue; // CHECK-MESSAGES-FROM: :[[@LINE]]:27: warning: implicit conversion 'bool' -> 'float' [readability-implicit-bool-conversion] + // CHECK-FIXES-FROM: float floatFromBool = static_cast<float>(boolValue); + + char charFromBool = boolValue; // CHECK-MESSAGES-FROM: :[[@LINE]]:25: warning: implicit conversion 'bool' -> 'char' [readability-implicit-bool-conversion] + // CHECK-FIXES-FROM: char charFromBool = static_cast<char>(boolValue); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
