https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/171070
>From 4ccd199ffecced16cfff596c771088ccbdc1c85f Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Thu, 4 Dec 2025 22:50:39 +0800 Subject: [PATCH 1/4] [clang-tidy] Fix implicit-bool-conversion false positives with C23 logical operators --- .../ImplicitBoolConversionCheck.cpp | 13 +++++-- .../readability/ImplicitBoolConversionCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../implicit-bool-conversion-c99.c | 25 +++++++++++++ .../readability/implicit-bool-conversion.c | 37 +++++++++++++------ 5 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index c135b426d8608..a5335bdb58c87 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -27,7 +27,9 @@ AST_MATCHER(Stmt, isMacroExpansion) { return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc); } -AST_MATCHER(Stmt, isC23) { return Finder->getASTContext().getLangOpts().C23; } +AST_MATCHER(Stmt, isC) { + return !Finder->getASTContext().getLangOpts().CPlusPlus; +} // Preserve same name as AST_MATCHER(isNULLMacroExpansion) // NOLINTNEXTLINE(llvm-prefer-static-over-anonymous-namespace) @@ -306,8 +308,8 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) { hasCastKind(CK_FloatingToBoolean), hasCastKind(CK_PointerToBoolean), hasCastKind(CK_MemberPointerToBoolean)), - // Exclude cases of C23 comparison result. - unless(allOf(isC23(), + // Exclude cases of C comparison result. + unless(allOf(isC(), hasSourceExpression(ignoringParens( binaryOperator(hasAnyOperatorName( ">", ">=", "==", "!=", "<", "<=")))))), @@ -350,6 +352,11 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) { unless(hasParent( binaryOperator(anyOf(BoolComparison, BoolXor, BoolOpAssignment, BitfieldAssignment)))), + // Exclude logical operators in C + unless(allOf(isC(), hasParent(binaryOperator( + hasAnyOperatorName("&&", "||"), + hasLHS(ImplicitCastFromBool), + hasRHS(ImplicitCastFromBool))))), implicitCastExpr().bind("implicitCastFromBool"), unless(hasParent(BitfieldConstruct)), // Check also for nested casts, for example: bool -> int -> float. diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h index 101089ccfb2e9..6ae15a9e19fe2 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h @@ -21,7 +21,7 @@ class ImplicitBoolConversionCheck : public ClangTidyCheck { public: ImplicitBoolConversionCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { - return LangOpts.Bool; + return LangOpts.Bool || LangOpts.C99; } void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 79a768e599cfd..a59f8942ceb8c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -543,7 +543,8 @@ Changes in existing checks - Improved :doc:`readability-implicit-bool-conversion <clang-tidy/checks/readability/implicit-bool-conversion>` check by correctly adding parentheses when the inner expression are implicitly converted - multiple times. + multiple times, enabling the check in C99, and avoiding false positives when + using logical operators with ``bool`` operands in C23. - Improved :doc:`readability-qualified-auto <clang-tidy/checks/readability/qualified-auto>` check by adding the option diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c new file mode 100644 index 0000000000000..dfadbbb5c3a61 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c @@ -0,0 +1,25 @@ +// RUN: %check_clang_tidy -std=c99 %s readability-implicit-bool-conversion %t + +typedef _Bool bool; +#define true 1 +#define false 0 + +bool returns_bool(void) { return true; } +int returns_int(void) { return 1; } + +void test_c99_logical_ops(void) { + bool b1 = true; + bool b2 = false; + + if (b1 && b2) {} + + if (b1 && returns_int()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion] + // CHECK-FIXES: if ((int)b1 && returns_int()) {} +} + +void test_c99_comparison(void) { + int x = 1; + int y = 2; + bool b = x > y; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c index 7f82b95f41799..d02f1b84d3dd6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c @@ -75,20 +75,10 @@ void implicitConversionFromBoolInComplexBoolExpressions() { bool anotherBoolean = false; int integer = boolean && anotherBoolean; - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion 'bool' -> 'int' - // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: implicit conversion 'bool' -> 'int' - // CHECK-FIXES: int integer = (int)boolean && (int)anotherBoolean; - float floating = (boolean || anotherBoolean) * 0.3f; - // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: implicit conversion 'bool' -> 'int' - // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: implicit conversion 'bool' -> 'int' - // CHECK-FIXES: float floating = ((int)boolean || (int)anotherBoolean) * 0.3f; - double doubleFloating = (boolean && (anotherBoolean || boolean)) * 0.3; // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit conversion 'bool' -> 'int' - // CHECK-MESSAGES: :[[@LINE-2]]:40: warning: implicit conversion 'bool' -> 'int' - // CHECK-MESSAGES: :[[@LINE-3]]:58: warning: implicit conversion 'bool' -> 'int' - // CHECK-FIXES: double doubleFloating = ((int)boolean && ((int)anotherBoolean || (int)boolean)) * 0.3; + // CHECK-FIXES: double doubleFloating = ((int)boolean && (anotherBoolean || boolean)) * 0.3; } void implicitConversionFromBoolLiterals() { @@ -371,3 +361,28 @@ int keepCompactReturnInC_PR71848() { // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion] // CHECK-FIXES: return(int)( foo ); } + +bool returns_bool(void) { return true; } + +void implicitConversionFromBoolInLogicalOps(int len) { + while (returns_bool()) {} + while ((len > 0) && returns_bool()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: while ((len > 0) && (int)returns_bool()) {} + while (returns_bool() && (len > 0)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: while ((int)returns_bool() && (len > 0)) {} + while ((len > 0) || returns_bool()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: while ((len > 0) || (int)returns_bool()) {} + while (returns_bool() || (len > 0)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: while ((int)returns_bool() || (len > 0)) {} +} + +void checkResultAssignment(void) { + int a = returns_bool() || returns_bool(); + bool b = returns_bool() && returns_bool(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' + // CHECK-FIXES: bool b = (returns_bool() && returns_bool()) != 0; +} >From 1a83b8ae5decd706ccf321ecb10855f8077ef7ea Mon Sep 17 00:00:00 2001 From: mitchell <[email protected]> Date: Mon, 8 Dec 2025 09:31:25 +0800 Subject: [PATCH 2/4] Update clang-tools-extra/docs/ReleaseNotes.rst Co-authored-by: EugeneZelenko <[email protected]> --- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a59f8942ceb8c..c00f6bb1d639f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -543,7 +543,7 @@ Changes in existing checks - Improved :doc:`readability-implicit-bool-conversion <clang-tidy/checks/readability/implicit-bool-conversion>` check by correctly adding parentheses when the inner expression are implicitly converted - multiple times, enabling the check in C99, and avoiding false positives when + multiple times, enabling the check in C99, and avoiding false positives when using logical operators with ``bool`` operands in C23. - Improved :doc:`readability-qualified-auto >From daef19c622464b285549dab6616f779933d6c0af Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Mon, 8 Dec 2025 13:33:10 +0800 Subject: [PATCH 3/4] Fix Matcher --- .../clang-tidy/readability/ImplicitBoolConversionCheck.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp index a5335bdb58c87..f1672579fa3fc 100644 --- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -28,7 +28,8 @@ AST_MATCHER(Stmt, isMacroExpansion) { } AST_MATCHER(Stmt, isC) { - return !Finder->getASTContext().getLangOpts().CPlusPlus; + return !Finder->getASTContext().getLangOpts().CPlusPlus && + !Finder->getASTContext().getLangOpts().ObjC; } // Preserve same name as AST_MATCHER(isNULLMacroExpansion) >From e9982a6eace6bcd1ddd2f612d3a078a3254622eb Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Mon, 8 Dec 2025 13:54:29 +0800 Subject: [PATCH 4/4] Add more testcases --- .../implicit-bool-conversion-c99.c | 31 ++++++++++++++++--- .../readability/implicit-bool-conversion.c | 14 +++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c index dfadbbb5c3a61..5a68e4e04512b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-c99.c @@ -1,15 +1,14 @@ // RUN: %check_clang_tidy -std=c99 %s readability-implicit-bool-conversion %t -typedef _Bool bool; #define true 1 #define false 0 -bool returns_bool(void) { return true; } +_Bool returns_bool(void) { return true; } int returns_int(void) { return 1; } void test_c99_logical_ops(void) { - bool b1 = true; - bool b2 = false; + _Bool b1 = true; + _Bool b2 = false; if (b1 && b2) {} @@ -21,5 +20,27 @@ void test_c99_logical_ops(void) { void test_c99_comparison(void) { int x = 1; int y = 2; - bool b = x > y; + _Bool b = x > y; +} + +void test_c99_native_keyword(void) { + _Bool raw_bool = 1; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion] + // CHECK-FIXES: _Bool raw_bool = true; + int i = raw_bool; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: int i = (int)raw_bool; +} + +void test_c99_macro_behavior(void) { + _Bool b = true; + int i = b + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicit conversion 'bool' -> 'int' + // CHECK-FIXES: int i = (int)b + 1; +} + +void test_c99_pointer_conversion(int *p) { + _Bool b = p; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: implicit conversion 'int *' -> 'bool' + // CHECK-FIXES: _Bool b = p != 0; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c index d02f1b84d3dd6..75952f979cb21 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c @@ -386,3 +386,17 @@ void checkResultAssignment(void) { // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicit conversion 'int' -> 'bool' // CHECK-FIXES: bool b = (returns_bool() && returns_bool()) != 0; } + +void checkNestedLogic(void) { + bool a = true; + bool b = false; + bool c = true; + + if ((a && b) || c) {} + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion] + // CHECK-FIXES: if ((a && b) || (int)c) {} + + if (c && (a || b)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicit conversion 'bool' -> 'int' [readability-implicit-bool-conversion] + // CHECK-FIXES: if ((int)c && (a || b)) {} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
