llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> Fixes https://github.com/llvm/llvm-project/issues/176889. --- Full diff: https://github.com/llvm/llvm-project/pull/189149.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp (+19) - (modified) clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h (+1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (modified) clang-tools-extra/docs/clang-tidy/checks/readability/implicit-bool-conversion.rst (+7) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-logical-operators.c (+94) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index a138d1900b799..31081f2f93dc2 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -245,11 +245,22 @@ static bool isCastAllowedInCondition(const ImplicitCastExpr *Cast, return false; } +static bool isLogicalOperatorResult(const ImplicitCastExpr *Cast) { + const Expr *SubExpr = Cast->getSubExpr()->IgnoreParenImpCasts(); + if (const auto *BinOp = dyn_cast<BinaryOperator>(SubExpr)) + return BinOp->isLogicalOp(); + if (const auto *UnOp = dyn_cast<UnaryOperator>(SubExpr)) + return UnOp->getOpcode() == UO_LNot; + return false; +} + ImplicitBoolConversionCheck::ImplicitBoolConversionCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), AllowIntegerConditions(Options.get("AllowIntegerConditions", false)), AllowPointerConditions(Options.get("AllowPointerConditions", false)), + AllowLogicalOperatorConversion( + Options.get("AllowLogicalOperatorConversion", false)), UseUpperCaseLiteralSuffix( Options.get("UseUpperCaseLiteralSuffix", false)) {} @@ -257,6 +268,8 @@ void ImplicitBoolConversionCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "AllowIntegerConditions", AllowIntegerConditions); Options.store(Opts, "AllowPointerConditions", AllowPointerConditions); + Options.store(Opts, "AllowLogicalOperatorConversion", + AllowLogicalOperatorConversion); Options.store(Opts, "UseUpperCaseLiteralSuffix", UseUpperCaseLiteralSuffix); } @@ -382,6 +395,12 @@ void ImplicitBoolConversionCheck::handleCastToBool(const ImplicitCastExpr *Cast, return; } + if (AllowLogicalOperatorConversion && + Cast->getCastKind() == CK_IntegralToBoolean && + isLogicalOperatorResult(Cast)) { + return; + } + auto Diag = diag(Cast->getBeginLoc(), "implicit conversion %0 -> 'bool'") << Cast->getSubExpr()->getType(); diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h index 6ae15a9e19fe2..add4b97c7bdfe 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h @@ -36,6 +36,7 @@ class ImplicitBoolConversionCheck : public ClangTidyCheck { const bool AllowIntegerConditions; const bool AllowPointerConditions; + const bool AllowLogicalOperatorConversion; const bool UseUpperCaseLiteralSuffix; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f8550e72dcc85..91e2b65d3ed12 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -402,6 +402,12 @@ Changes in existing checks temporaries (e.g. passing a string literal to a ``const std::string&`` parameter) +- Improved :doc:`readability-implicit-bool-conversion + <clang-tidy/checks/readability/implicit-bool-conversion>` check by adding + `AllowLogicalOperatorConversion` option to suppress warnings on implicit + conversions of logical operator results (``&&``, ``||``, ``!``) to ``bool`` + in C. + - Improved :doc:`readability-non-const-parameter <clang-tidy/checks/readability/non-const-parameter>` check by avoiding false positives on parameters used in dependent expressions (e.g. inside generic 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 66e2fd0beaea1..efa30ae8a4649 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 @@ -139,6 +139,13 @@ Options When `true`, the check will allow conditional pointer conversions. Default is `false`. +.. option:: AllowLogicalOperatorConversion + + When `true`, the check will suppress warnings for implicit conversions of + logical operator results (``&&``, ``||``, ``!``) to ``bool``. These + operators always produce values equal to ``0`` or ``1``, so the conversion + is safe. Default is `false`. + .. option:: UseUpperCaseLiteralSuffix When `true`, the replacements will use an uppercase literal suffix in the diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-logical-operators.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-logical-operators.c new file mode 100644 index 0000000000000..4b4a8e8de76da --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-logical-operators.c @@ -0,0 +1,94 @@ +// RUN: %check_clang_tidy -std=c23-or-later %s readability-implicit-bool-conversion %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: readability-implicit-bool-conversion.AllowLogicalOperatorConversion: true \ +// RUN: }}' + +void function_taking_bool(bool); +bool returns_bool(void); +int returns_int(void); + +void logical_or_to_bool(void) { + bool a = true, b = false; + bool c = a || b; + bool d = returns_bool() || returns_bool(); + bool e = returns_bool() || (a && b); +} + +void logical_and_to_bool(void) { + bool a = true, b = false; + bool c = a && b; + bool d = returns_bool() && returns_bool(); +} + +void logical_not_to_bool(void) { + bool a = true; + int x = 5; + bool b = !a; + bool c = !x; +} + +void logical_with_literals(void) { + bool a = true, b = false; + bool c = true || b; + bool d = false || b; + bool e = a || true; + bool f = false || (a || b); +} + +void nested_logical_ops(void) { + bool a = true, b = false, c = true; + bool d = (a && b) || c; + bool e = a || (b && c); + bool f = !(a || b); +} + +void logical_in_function_call(void) { + bool a = true, b = false; + function_taking_bool(a || b); + function_taking_bool(a && b); + function_taking_bool(!a); +} + +bool logical_in_return(void) { + bool a = true, b = false; + return a || b; +} + +void still_warn_on_regular_int_to_bool(void) { + int x = 42; + bool b = x; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion] + // CHECK-FIXES: bool b = x != 0; + + bool c = x + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' + // CHECK-FIXES: bool c = (x + 1) != 0; + + bool d = returns_int(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' + // CHECK-FIXES: bool d = returns_int() != 0; +} + +void still_warn_on_bitwise_ops(void) { + int x = 5, y = 3; + bool b = x | y; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' + // CHECK-FIXES: bool b = (x | y) != 0; + + bool c = x & y; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' + // CHECK-FIXES: bool c = (x & y) != 0; +} + +void cast_from_bool_still_warns(void) { + bool a = true; + int x = a; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: int x = (int)a; +} + +void comparison_still_excluded(void) { + bool b1 = 1 > 0; + bool b2 = 1 == 0; + bool b3 = 1 < 2; +} `````````` </details> https://github.com/llvm/llvm-project/pull/189149 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
