Author: NAKAMURA Takumi Date: 2024-12-28T17:47:00+09:00 New Revision: 537d4e9d21be1f5e40a780f570663b04572765af
URL: https://github.com/llvm/llvm-project/commit/537d4e9d21be1f5e40a780f570663b04572765af DIFF: https://github.com/llvm/llvm-project/commit/537d4e9d21be1f5e40a780f570663b04572765af.diff LOG: Revert "Added options to readability-implicit-bool-conversion (#120087)" This reverts commit 5bec2b71b44ddff44aa4d8534b58a5561389bb1d. (llvmorg-20-init-16425-g5bec2b71b44d) This broke tests. Added: 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: clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-check.cpp ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index 48851da143068f..f9fd1d903e231e 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -259,17 +259,13 @@ ImplicitBoolConversionCheck::ImplicitBoolConversionCheck( AllowIntegerConditions(Options.get("AllowIntegerConditions", false)), AllowPointerConditions(Options.get("AllowPointerConditions", false)), UseUpperCaseLiteralSuffix( - Options.get("UseUpperCaseLiteralSuffix", false)), - CheckConversionsToBool(Options.get("CheckConversionsToBool", true)), - CheckConversionsFromBool(Options.get("CheckConversionsFromBool", true)) {} + Options.get("UseUpperCaseLiteralSuffix", false)) {} 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) { @@ -281,7 +277,6 @@ 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. @@ -292,84 +287,72 @@ 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()))); - 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); - } + 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); } void ImplicitBoolConversionCheck::check( diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h index b0c3c2943e649c..5947f7316e67cc 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h @@ -37,8 +37,6 @@ 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 8c360222ce43da..fabd0cc78ac645 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -342,11 +342,10 @@ 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, and by adding the option `CheckConversionsToBool` or - `CheckConversionsFromBool` to configure checks for conversions involving ``bool``. + <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. - 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 f7c15ffa2da51b..88cff387f4c165 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,41 +147,3 @@ 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 deleted file mode 100644 index 8ba4635de1704f..00000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-check.cpp +++ /dev/null @@ -1,57 +0,0 @@ -// 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
