[clang-tools-extra] 3bf322e - [clang-tidy] Add bugprone-non-zero-enum-to-bool-conversion check
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 16 01:51:21 PDT 2023
Author: Piotr Zegar
Date: 2023-04-16T08:51:00Z
New Revision: 3bf322e69d5cb8c74c71d55d00caad7c95c5270b
URL: https://github.com/llvm/llvm-project/commit/3bf322e69d5cb8c74c71d55d00caad7c95c5270b
DIFF: https://github.com/llvm/llvm-project/commit/3bf322e69d5cb8c74c71d55d00caad7c95c5270b.diff
LOG: [clang-tidy] Add bugprone-non-zero-enum-to-bool-conversion check
Detect implicit and explicit conversions of enum to bool,
when enum doesn't have a enumerator with value equal to 0.
In theory such conversion should always return TRUE.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D144036
Added:
clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion-cpp11.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 11b4051668cef..60666287cf307 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -38,6 +38,7 @@
#include "MoveForwardingReferenceCheck.h"
#include "MultipleStatementMacroCheck.h"
#include "NoEscapeCheck.h"
+#include "NonZeroEnumToBoolConversionCheck.h"
#include "NotNullTerminatedResultCheck.h"
#include "ParentVirtualCallCheck.h"
#include "PosixReturnCheck.h"
@@ -139,6 +140,8 @@ class BugproneModule : public ClangTidyModule {
CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
"bugprone-narrowing-conversions");
CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
+ CheckFactories.registerCheck<NonZeroEnumToBoolConversionCheck>(
+ "bugprone-non-zero-enum-to-bool-conversion");
CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
"bugprone-not-null-terminated-result");
CheckFactories.registerCheck<ParentVirtualCallCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index a975c86fc3970..5af5a59e5340f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -33,17 +33,18 @@ add_clang_library(clangTidyBugproneModule
MoveForwardingReferenceCheck.cpp
MultipleStatementMacroCheck.cpp
NoEscapeCheck.cpp
+ NonZeroEnumToBoolConversionCheck.cpp
NotNullTerminatedResultCheck.cpp
ParentVirtualCallCheck.cpp
PosixReturnCheck.cpp
RedundantBranchConditionCheck.cpp
ReservedIdentifierCheck.cpp
SharedPtrArrayMismatchCheck.cpp
- SmartPtrArrayMismatchCheck.cpp
SignalHandlerCheck.cpp
SignedCharMisuseCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
+ SmartPtrArrayMismatchCheck.cpp
SpuriouslyWakeUpFunctionsCheck.cpp
StandaloneEmptyCheck.cpp
StringConstructorCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.cpp
new file mode 100644
index 0000000000000..a59c4fc47c0b4
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.cpp
@@ -0,0 +1,78 @@
+//===--- NonZeroEnumToBoolConversionCheck.cpp - clang-tidy ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "NonZeroEnumToBoolConversionCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) {
+ const EnumDecl *Definition = Node.getDefinition();
+ return Definition && Node.isComplete() &&
+ std::none_of(Definition->enumerator_begin(),
+ Definition->enumerator_end(),
+ [](const EnumConstantDecl *Value) {
+ return Value->getInitVal().isZero();
+ });
+}
+
+} // namespace
+
+NonZeroEnumToBoolConversionCheck::NonZeroEnumToBoolConversionCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ EnumIgnoreList(
+ utils::options::parseStringList(Options.get("EnumIgnoreList", ""))) {}
+
+void NonZeroEnumToBoolConversionCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "EnumIgnoreList",
+ utils::options::serializeStringList(EnumIgnoreList));
+}
+
+bool NonZeroEnumToBoolConversionCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus;
+}
+
+void NonZeroEnumToBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ castExpr(hasCastKind(CK_IntegralToBoolean),
+ unless(isExpansionInSystemHeader()), hasType(booleanType()),
+ hasSourceExpression(
+ expr(hasType(qualType(hasCanonicalType(hasDeclaration(
+ enumDecl(isCompleteAndHasNoZeroValue(),
+ unless(matchers::matchesAnyListedName(
+ EnumIgnoreList)))
+ .bind("enum"))))),
+ unless(declRefExpr(to(enumConstantDecl()))))),
+ unless(hasAncestor(staticAssertDecl())))
+ .bind("cast"),
+ this);
+}
+
+void NonZeroEnumToBoolConversionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Cast = Result.Nodes.getNodeAs<CastExpr>("cast");
+ const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum");
+
+ diag(Cast->getExprLoc(), "conversion of %0 into 'bool' will always return "
+ "'true', enum doesn't have a zero-value enumerator")
+ << Enum;
+ diag(Enum->getLocation(), "enum is defined here", DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.h b/clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.h
new file mode 100644
index 0000000000000..f1cb81f05a723
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.h
@@ -0,0 +1,36 @@
+//===--- NonZeroNonZeroEnumToBoolConversionCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NONZEROENUMTOBOOLCONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NONZEROENUMTOBOOLCONVERSIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+/// Detect implicit and explicit casts of `enum` type into `bool` where
+/// `enum` type doesn't have a zero-value enumerator.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion.html
+class NonZeroEnumToBoolConversionCheck : public ClangTidyCheck {
+public:
+ NonZeroEnumToBoolConversionCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+private:
+ const std::vector<StringRef> EnumIgnoreList;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NONZEROENUMTOBOOLCONVERSIONCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c19636b95c555..61cd67c1745f9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,6 +106,12 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-non-zero-enum-to-bool-conversion
+ <clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion>` check.
+
+ Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum`` type
+ doesn't have a zero-value enumerator.
+
- New :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion.rst
new file mode 100644
index 0000000000000..168ed71674773
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion.rst
@@ -0,0 +1,53 @@
+.. title:: clang-tidy - bugprone-non-zero-enum-to-bool-conversion
+
+bugprone-non-zero-enum-to-bool-conversion
+=========================================
+
+Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum``
+type doesn't have a zero-value enumerator. If the ``enum`` is used only to hold
+values equal to its enumerators, then conversion to ``bool`` will always result
+in ``true`` value. This can lead to unnecessary code that reduces readability
+and maintainability and can result in bugs.
+
+May produce false positives if the ``enum`` is used to store other values
+(used as a bit-mask or zero-initialized on purpose). To deal with them,
+``// NOLINT`` or casting first to the underlying type before casting to ``bool``
+can be used.
+
+It is important to note that this check will not generate warnings if the
+definition of the enumeration type is not available.
+Additionally, C++11 enumeration classes are supported by this check.
+
+Overall, this check serves to improve code quality and readability by identifying
+and flagging instances where implicit or explicit casts from enumeration types to
+boolean could cause potential issues.
+
+Example
+-------
+
+.. code-block:: c++
+
+ enum EStatus {
+ OK = 1,
+ NOT_OK,
+ UNKNOWN
+ };
+
+ void process(EStatus status) {
+ if (!status) {
+ // this true-branch won't be executed
+ return;
+ }
+ // proceed with "valid data"
+ }
+
+Options
+-------
+
+.. option:: EnumIgnoreList
+
+ Option is used to ignore certain enum types when checking for
+ implicit/explicit casts to bool. It accepts a semicolon-separated list of
+ (fully qualified) enum type names or regular expressions that match the enum
+ type names.
+ The default value is an empty string, which means no enums will be ignored.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c4eaedc253f8e..38727f69ea564 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -104,6 +104,7 @@ Clang-Tidy Checks
`bugprone-move-forwarding-reference <bugprone/move-forwarding-reference.html>`_, "Yes"
`bugprone-multiple-statement-macro <bugprone/multiple-statement-macro.html>`_,
`bugprone-no-escape <bugprone/no-escape.html>`_,
+ `bugprone-non-zero-enum-to-bool-conversion <bugprone/non-zero-enum-to-bool-conversion.html>`_,
`bugprone-not-null-terminated-result <bugprone/not-null-terminated-result.html>`_, "Yes"
`bugprone-parent-virtual-call <bugprone/parent-virtual-call.html>`_, "Yes"
`bugprone-posix-return <bugprone/posix-return.html>`_, "Yes"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion-cpp11.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion-cpp11.cpp
new file mode 100644
index 0000000000000..42927c615ffc7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion-cpp11.cpp
@@ -0,0 +1,110 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-non-zero-enum-to-bool-conversion %t
+
+namespace with::issue {
+
+enum class EStatusC : char {
+ SUCCESS = 1,
+ FAILURE = 2,
+ INVALID_PARAM = 3,
+ UNKNOWN = 4
+};
+
+bool testEnumConversion(EStatusC value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EStatusC' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+enum class EStatusS : short {
+ SUCCESS = 1,
+ FAILURE = 2,
+ INVALID_PARAM = 3,
+ UNKNOWN = 4
+};
+
+bool testEnumConversion(EStatusS value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EStatusS' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+
+enum class EStatusI : int {
+ SUCCESS = 1,
+ FAILURE = 2,
+ INVALID_PARAM = 3,
+ UNKNOWN = 4
+};
+
+bool testEnumConversion(EStatusI value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EStatusI' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+enum class EStatus {
+ SUCCESS = 1,
+ FAILURE = 2,
+ INVALID_PARAM = 3,
+ UNKNOWN = 4
+};
+
+bool testEnumConversion(EStatus value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+namespace enum_int {
+
+enum EResult : int {
+ OK = 1,
+ NOT_OK
+};
+
+bool testEnumConversion(const EResult& value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EResult' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return value;
+}
+
+}
+
+namespace enum_short {
+
+enum EResult : short {
+ OK = 1,
+ NOT_OK
+};
+
+bool testEnumConversion(const EResult& value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EResult' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return value;
+}
+
+}
+
+namespace enum_char {
+
+enum EResult : char {
+ OK = 1,
+ NOT_OK
+};
+
+bool testEnumConversion(const EResult& value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EResult' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return value;
+}
+
+}
+
+namespace enum_default {
+
+enum EResult {
+ OK = 1,
+ NOT_OK
+};
+
+bool testEnumConversion(const EResult& value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EResult' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return value;
+}
+
+}
+
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion.cpp
new file mode 100644
index 0000000000000..00f2e71229895
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/non-zero-enum-to-bool-conversion.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-non-zero-enum-to-bool-conversion %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-non-zero-enum-to-bool-conversion.EnumIgnoreList, value: '::without::issue::IgnoredEnum;IgnoredSecondEnum'}]}"
+
+namespace with::issue {
+
+typedef enum EStatus {
+ SUCCESS = 1,
+ FAILURE = 2,
+ INVALID_PARAM = 3,
+ UNKNOWN = 4
+} Status;
+
+bool testEnumConversion(EStatus value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return value;
+}
+
+bool testTypedefConversion(Status value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return value;
+}
+
+bool testExplicitConversion(EStatus value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:28: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+bool testInIfConversion(EStatus value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:7: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ if (value) {
+ return false;
+ }
+ return true;
+}
+
+bool testWithNegation(EStatus value) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:14: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-non-zero-enum-to-bool-conversion]
+ return not value;
+}
+
+}
+
+namespace without::issue {
+
+enum StatusWithZero {
+ UNK = 0,
+ OK = 1,
+ NOT_OK = 2
+};
+
+bool testEnumConversion(StatusWithZero value) {
+ return value;
+}
+
+enum WithDefault {
+ Value0,
+ Value1
+};
+
+bool testEnumConversion(WithDefault value) {
+ return value;
+}
+
+enum WithNegative : int {
+ Nen2 = -2,
+ Nen1,
+ Nen0
+};
+
+bool testEnumConversion(WithNegative value) {
+ return value;
+}
+
+enum EStatus {
+ SUCCESS = 1,
+ FAILURE,
+ INVALID_PARAM,
+ UNKNOWN
+};
+
+bool explicitCompare(EStatus value) {
+ return value == SUCCESS;
+}
+
+bool testEnumeratorCompare() {
+ return SUCCESS;
+}
+
+enum IgnoredEnum {
+ IGNORED_VALUE_1 = 1,
+ IGNORED_VALUE_2
+};
+
+enum IgnoredSecondEnum {
+ IGNORED_SECOND_VALUE_1 = 1,
+ IGNORED_SECOND_VALUE_2
+};
+
+bool testIgnored(IgnoredEnum value) {
+ return value;
+}
+
+bool testIgnored(IgnoredSecondEnum value) {
+ return value;
+}
+
+}
More information about the cfe-commits
mailing list