Skip to content
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
03f5368
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 16, 2024
16c7c95
Update ImplicitBoolConversionCheck.h
4m4n-x-B4w4ne Dec 16, 2024
0d6fae8
Create implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 16, 2024
8f134a3
Merge branch 'main' into main
4m4n-x-B4w4ne Dec 16, 2024
e13bf5a
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 17, 2024
a480460
Update implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 17, 2024
5d1e7fc
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
391c870
Merge branch 'llvm:main' into main
4m4n-x-B4w4ne Dec 18, 2024
9e83c0d
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
4b75aff
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
dc67c7f
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
c6078dc
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
0882713
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
d59f000
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
4520dd6
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
a9376ea
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 18, 2024
dc229ca
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 18, 2024
f6efcaa
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 18, 2024
ac54d56
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 18, 2024
74031e5
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 18, 2024
9b46c09
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 18, 2024
dd41e29
Update implicit-bool-conversion.rst
4m4n-x-B4w4ne Dec 18, 2024
d0c0593
Update implicit-bool-conversion.rst
4m4n-x-B4w4ne Dec 18, 2024
a56019f
Update implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 18, 2024
ed6e001
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 19, 2024
4900886
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 19, 2024
e1eaf30
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 19, 2024
899683c
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 20, 2024
414768e
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 20, 2024
565bef8
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 20, 2024
dac44f4
Update implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 20, 2024
fd1f7dc
[Test]Update implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 20, 2024
df5e48e
Update implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 22, 2024
56a4e09
Update implicit-bool-conversion-check.cpp
4m4n-x-B4w4ne Dec 22, 2024
75fe805
Update ReleaseNotes.rst
4m4n-x-B4w4ne Dec 23, 2024
dff9123
Update ImplicitBoolConversionCheck.cpp
4m4n-x-B4w4ne Dec 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -287,72 +292,83 @@ 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
C23 , and by adding the option `CheckConversionsToBool` or
C23, and by adding the option `CheckConversionsToBool` or

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, What needs to be done further? Can you please help me out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix Clang-format complains.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done Thanks.

`CheckConversionsFromBool` to configure checks for conversions involving ``bool``.

- Improved :doc:`readability-redundant-smartptr-get
<clang-tidy/checks/readability/redundant-smartptr-get>` check to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have added -check-suffix options to each of your RUN lines, but none of your CHECK-MESSAGES are actually using it. E.g.,:

// CHECK-MESSAGES-TO-BOOL-FALSE

Although, instead of using the suffixes to say FALSE, you could swap them around to say that the option is enabled, so -check-suffix:TO-BOOL. Then, the version with both enabled becomes -check-suffix:TO-BOOL,FROM-BOOL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ,I am working on it and also on clean PR build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have wrote a basic-level test similar to other checks and also have done a clean PR build.
Can you please review it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have written check file for all permutations of configuration of the options and also for the more comprehensive test cases as instructed with clean PR build.
Done Thanks.
@5chmidti Can you please review it?

Original file line number Diff line number Diff line change
@@ -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);
}