Skip to content

[clang-tidy] Exclude bitwise operators in bugprone-non-zero-enum-to-bool-conversion #65498

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -49,6 +49,11 @@ bool NonZeroEnumToBoolConversionCheck::isLanguageVersionSupported(
}

void NonZeroEnumToBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
// Excluding bitwise operators (binary and overload) to avoid false-positives
// in code like this 'if (e & SUCCESS) {'.
auto ExcludedOperators = binaryOperation(hasAnyOperatorName(
"|", "&", "^", "<<", ">>", "~", "|=", "&=", "^=", "<<=", ">>="));
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want, then yes, I can reduce this back to overload operators calls only.

I think current implement it fine, but maybe add some comment here to clarify why you want to exclude those operators. Because actually it's for the operator overloading instead of explicit operator.


Finder->addMatcher(
castExpr(hasCastKind(CK_IntegralToBoolean),
unless(isExpansionInSystemHeader()), hasType(booleanType()),
Expand All @@ -58,7 +63,8 @@ void NonZeroEnumToBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
unless(matchers::matchesAnyListedName(
EnumIgnoreList)))
.bind("enum"))))),
unless(declRefExpr(to(enumConstantDecl()))))),
unless(declRefExpr(to(enumConstantDecl()))),
unless(ignoringImplicit(ExcludedOperators)))),
unless(hasAncestor(staticAssertDecl())))
.bind("cast"),
this);
Expand Down
6 changes: 5 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ Changes in existing checks
<clang-tidy/checks/bugprone/lambda-function-name>` check by adding option
`IgnoreMacros` to ignore warnings in macros.

- Improved :doc:`bugprone-non-zero-enum-to-bool-conversion
<clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion>` check by
eliminating false positives resulting from direct usage of bitwise operators.

- Improved :doc:`bugprone-reserved-identifier
<clang-tidy/checks/bugprone/reserved-identifier>` check, so that it does not
warn on macros starting with underscore and lowercase letter.
Expand Down Expand Up @@ -227,7 +231,7 @@ Changes in existing checks
`DeduplicateFindings` to output one finding per symbol occurrence.

- Improved :doc:`misc-include-cleaner
<clang-tidy/checks/misc/include-cleaner>` check to avoid fixes insert
<clang-tidy/checks/misc/include-cleaner>` check to avoid fixes insert
same include header multiple times.

- Improved :doc:`misc-redundant-expression
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ bool explicitCompare(EStatus value) {
return value == SUCCESS;
}

bool explicitBitUsage1(EStatus value) {
return (value & SUCCESS);
}

bool explicitBitUsage2(EStatus value) {
return (value | SUCCESS);
}

bool testEnumeratorCompare() {
return SUCCESS;
}
Expand All @@ -104,4 +112,16 @@ bool testIgnored(IgnoredSecondEnum value) {
return value;
}

enum CustomOperatorEnum {
E0 = 0x1,
E1 = 0x2,
E2 = 0x4
};

CustomOperatorEnum operator&(CustomOperatorEnum a, CustomOperatorEnum b) { return static_cast<CustomOperatorEnum>(a & b); }

void testCustomOperator(CustomOperatorEnum e) {
if (e & E1) {}
}

}