-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang-tidy] fix false positive for implicit conversion of comparison result in C23 #113639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… result in C23 Fixed llvm#111013 bool will be builtin type in C23 but comparison result in C is still int. It is no need to change this kind of implicit cast to explicit cast.
|
@llvm/pr-subscribers-clang-tools-extra Author: Congcong Cai (HerrCai0907) ChangesFixed #111013 Full diff: https://github.com/llvm/llvm-project/pull/113639.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 968a4a55a6d798..06415c1346a4f0 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -26,6 +26,8 @@ AST_MATCHER(Stmt, isMacroExpansion) {
return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc);
}
+AST_MATCHER(Stmt, isC23) { return Finder->getASTContext().getLangOpts().C23; }
+
bool isNULLMacroExpansion(const Stmt *Statement, ASTContext &Context) {
SourceManager &SM = Context.getSourceManager();
const LangOptions &LO = Context.getLangOpts();
@@ -298,6 +300,11 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
hasCastKind(CK_FloatingToBoolean),
hasCastKind(CK_PointerToBoolean),
hasCastKind(CK_MemberPointerToBoolean)),
+ // Exclude cases of C23 comparison result.
+ unless(allOf(
+ isC23(),
+ hasSourceExpression(binaryOperator(hasAnyOperatorName(
+ ">", ">=", "==", "!=", "<", "<="))))),
// Exclude case of using if or while statements with variable
// declaration, e.g.:
// if (int var = functionCall()) {}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a9b1ab367f538a..e42082806fd308 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -244,7 +244,8 @@ Changes in existing checks
- 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.
+ 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/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
index f3dc32c10d640a..0b231d10adf8fc 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
@@ -304,6 +304,15 @@ void implicitConversionToBoolFromUnaryMinusAndZeroLiterals() {
// CHECK-FIXES: functionTakingBool((-0.0) != 0.0);
}
+void ignoreImplicitCastToBoolForComparisonResult() {
+ bool boolFromComparison0 = 1 != 0;
+ bool boolFromComparison1 = 1 == 0;
+ bool boolFromComparison2 = 1 > 0;
+ bool boolFromComparison3 = 1 >= 0;
+ bool boolFromComparison4 = 1 < 0;
+ bool boolFromComparison5 = 1 <= 0;
+}
+
void ignoreExplicitCastsToBool() {
int integer = 10;
bool boolComingFromInt = (bool)integer;
|
|
@llvm/pr-subscribers-clang-tidy Author: Congcong Cai (HerrCai0907) ChangesFixed #111013 Full diff: https://github.com/llvm/llvm-project/pull/113639.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 968a4a55a6d798..06415c1346a4f0 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -26,6 +26,8 @@ AST_MATCHER(Stmt, isMacroExpansion) {
return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc);
}
+AST_MATCHER(Stmt, isC23) { return Finder->getASTContext().getLangOpts().C23; }
+
bool isNULLMacroExpansion(const Stmt *Statement, ASTContext &Context) {
SourceManager &SM = Context.getSourceManager();
const LangOptions &LO = Context.getLangOpts();
@@ -298,6 +300,11 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
hasCastKind(CK_FloatingToBoolean),
hasCastKind(CK_PointerToBoolean),
hasCastKind(CK_MemberPointerToBoolean)),
+ // Exclude cases of C23 comparison result.
+ unless(allOf(
+ isC23(),
+ hasSourceExpression(binaryOperator(hasAnyOperatorName(
+ ">", ">=", "==", "!=", "<", "<="))))),
// Exclude case of using if or while statements with variable
// declaration, e.g.:
// if (int var = functionCall()) {}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a9b1ab367f538a..e42082806fd308 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -244,7 +244,8 @@ Changes in existing checks
- 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.
+ 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/test/clang-tidy/checkers/readability/implicit-bool-conversion.c b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.c
index f3dc32c10d640a..0b231d10adf8fc 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
@@ -304,6 +304,15 @@ void implicitConversionToBoolFromUnaryMinusAndZeroLiterals() {
// CHECK-FIXES: functionTakingBool((-0.0) != 0.0);
}
+void ignoreImplicitCastToBoolForComparisonResult() {
+ bool boolFromComparison0 = 1 != 0;
+ bool boolFromComparison1 = 1 == 0;
+ bool boolFromComparison2 = 1 > 0;
+ bool boolFromComparison3 = 1 >= 0;
+ bool boolFromComparison4 = 1 < 0;
+ bool boolFromComparison5 = 1 <= 0;
+}
+
void ignoreExplicitCastsToBool() {
int integer = 10;
bool boolComingFromInt = (bool)integer;
|
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
Outdated
Show resolved
Hide resolved
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/3266 Here is the relevant piece of the build log for the reference |
… result in C23 (llvm#113639) Fixed llvm#111013 bool will be builtin type in C23 but comparison result in C is still int. It is no need to change this kind of implicit cast to explicit cast.
Fixed #111013
bool will be builtin type in C23 but comparison result in C is still int.
It is no need to change this kind of implicit cast to explicit cast.