Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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 @@ -258,14 +258,17 @@ ImplicitBoolConversionCheck::ImplicitBoolConversionCheck(
: ClangTidyCheck(Name, Context),
AllowIntegerConditions(Options.get("AllowIntegerConditions", false)),
AllowPointerConditions(Options.get("AllowPointerConditions", false)),
UseUpperCaseLiteralSuffix(
Options.get("UseUpperCaseLiteralSuffix", false)) {}
UseUpperCaseLiteralSuffix(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 Down Expand Up @@ -357,20 +360,22 @@ void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) {

void ImplicitBoolConversionCheck::check(
const MatchFinder::MatchResult &Result) {

if(CheckConversionsToBool){
Copy link
Member

Choose a reason for hiding this comment

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

Wrong, place these things should be actually put into matcher in registerMatchers method, to speed up.

You got 2 calls to addMatcher, one in line 300, other in line 339, put those (with dependences to avoid unused local variables) into if's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay Thanks I am Working on 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.

Can You please check this? I have done this now as instructed.

if (const auto *CastToBool =
Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastToBool")) {
const auto *Parent = Result.Nodes.getNodeAs<Stmt>("parentStmt");
return handleCastToBool(CastToBool, Parent, *Result.Context);
}

if (const auto *CastFromBool =
}
if(CheckConversionsFromBool ){
if (const auto *CastFromBool =
Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastFromBool")) {
const auto *NextImplicitCast =
const auto *NextImplicitCast =
Result.Nodes.getNodeAs<ImplicitCastExpr>("furtherImplicitCast");
return handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context);
return handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context);
}
}
}

void ImplicitBoolConversionCheck::handleCastToBool(const ImplicitCastExpr *Cast,
const Stmt *Parent,
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Belongs to previous line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but i havent got this one . Can you please clarify?

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 got it now.

- `UseUpperCaseLiteralSuffix` : to select the case of the literal suffix in fixes and fixing false positive for implicit conversion of comparison result in C23.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow 80-characters limit.

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.

- `CheckConversionsToBool`: to allow enabling or disabling warnings for implicit conversions to `bool` .
- `CheckConversionsFromBool`: to allow enabling or disabling warnings for implicit conversions from `bool`.

- Improved :doc:`readability-redundant-smartptr-get
<clang-tidy/checks/readability/redundant-smartptr-get>` check to
Expand Down
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,86 @@
// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t

// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t \
// RUN: -- -config='{CheckOptions: [{key: readability-implicit-bool-conversion.CheckConversionsToBool, value: false}, {key: readability-implicit-bool-conversion.CheckConversionsFromBool, value: true}]}'
Copy link
Member

Choose a reason for hiding this comment

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

this is old format for options, use new one like in other checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay Thanks I am Working on 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 done this . Can you please review it?


// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t \
// RUN: -- -config='{CheckOptions: [{key: readability-implicit-bool-conversion.CheckConversionsToBool, value: true}, {key: readability-implicit-bool-conversion.CheckConversionsFromBool, value: false}]}'

// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t \
// RUN: -- -config='{CheckOptions: [{key: readability-implicit-bool-conversion.CheckConversionsToBool, value: false}, {key: readability-implicit-bool-conversion.CheckConversionsFromBool, value: false}]}'

// ==========================================================
// Test Case: Conversions to bool (CheckConversionsToBool=true)
// ==========================================================
void TestConversionsToBool() {
int x = 42;
if (x) // CHECK-MESSAGES: :[[@LINE]]:8: warning: implicit conversion 'int' -> 'bool'
(void)0;

float f = 3.14;
if (f) // CHECK-MESSAGES: :[[@LINE]]:8: warning: implicit conversion 'float' -> 'bool'
(void)0;

int *p = nullptr;
if (p) // CHECK-MESSAGES: :[[@LINE]]:8: warning: implicit conversion 'int *' -> 'bool'
(void)0;

// Pointer-to-member
struct S {
int member;
};
int S::*ptr = nullptr;
if (ptr) // CHECK-MESSAGES: :[[@LINE]]:8: warning: implicit conversion 'int S::*' -> 'bool'
(void)0;
}

// ==========================================================
// Test Case: Conversions from bool (CheckConversionsFromBool=true)
// ==========================================================
void TestConversionsFromBool() {
bool b = true;

int x = b; // CHECK-MESSAGES: :[[@LINE]]:12: warning: implicit conversion 'bool' -> 'int'
float f = b; // CHECK-MESSAGES: :[[@LINE]]:12: warning: implicit conversion 'bool' -> 'float'
}

// ==========================================================
// Test Case: Mixed Configurations (ToBool=false, FromBool=true)
// ==========================================================
void TestMixedConfig() {
int x = 42;
if (x) // No warning: CheckConversionsToBool=false
(void)0;

bool b = true;
int y = b; // CHECK-MESSAGES: :[[@LINE]]:12: warning: implicit conversion 'bool' -> 'int'
}

// ==========================================================
// Test Case: No Diagnostics (ToBool=false, FromBool=false)
// ==========================================================
void TestNoDiagnostics() {
int x = 42;
if (x) // No warning: CheckConversionsToBool=false
(void)0;

bool b = true;
int y = b; // No warning: CheckConversionsFromBool=false
}

// ==========================================================
// Test Case: Edge Cases and Complex Expressions
// ==========================================================
void TestEdgeCases() {
bool b = true;

// Nested implicit casts
int x = (b ? 1 : 0); // CHECK-MESSAGES: :[[@LINE]]:12: warning: implicit conversion 'bool' -> 'int'

// Function returns implicit bool
auto ReturnBool = []() -> bool { return true; };
int y = ReturnBool(); // CHECK-MESSAGES: :[[@LINE]]:12: warning: implicit conversion 'bool' -> 'int'

// Explicit casts (no diagnostics)
int z = static_cast<int>(b); // No warning: explicit cast
}