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

4m4n-x-B4w4ne · web-flow · commit 5bec2b71b44d · 2024-12-28T15:35:30.000+08:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 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);
+}
