Revert "Added options to readability-implicit-bool-conversion (#120087)"

This reverts commit 5bec2b71b44ddff44aa4d8534b58a5561389bb1d. (llvmorg-20-init-16425-g5bec2b71b44d) This broke tests.
2024-12-28 17:47:00 +09:00 · 2024-12-28 17:47:00 +09:00 · 537d4e9d21
commit 537d4e9d21
parent 5bec2b71b4
5 changed files with 67 additions and 182 deletions
--- 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);

-    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);
-  }
+  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(
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
@ -37,8 +37,6 @@ private:
  const bool AllowIntegerConditions;
  const bool AllowPointerConditions;
  const bool UseUpperCaseLiteralSuffix;
-  const bool CheckConversionsToBool;
-  const bool CheckConversionsFromBool;
 };

 } // namespace clang::tidy::readability
--- 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
--- 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;
-
--- 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
@ -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 different 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);
-}