PiotrZSL updated this revision to Diff 502752.
PiotrZSL added a comment.
Ping, Rebase, Changed Option from Regexp to List, Improved documentation,
Removed usage of deprecated API
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144036/new/
https://reviews.llvm.org/D144036
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion-cpp11.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion.cpp
@@ -0,0 +1,130 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-enum-to-bool-conversion %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-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]]:12: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-enum-to-bool-conversion]
+ return value;
+}
+
+bool testTypedefConversion(Status value)
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:12: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-enum-to-bool-conversion]
+ return value;
+}
+
+bool testExplicitConversion(EStatus value)
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+bool testInIfConversion(EStatus value)
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:9: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-enum-to-bool-conversion]
+ if (value)
+ {
+ return false;
+ }
+
+ return true;
+}
+
+bool testWithNegation(EStatus value)
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:16: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-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;
+}
+
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion-cpp11.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion-cpp11.cpp
@@ -0,0 +1,32 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-enum-to-bool-conversion %t
+
+namespace with::issue
+{
+
+enum class EStatus : char
+{
+ SUCCESS = 1,
+ FAILURE = 2,
+ INVALID_PARAM = 3,
+ UNKNOWN = 4
+};
+
+bool testEnumConversion(EStatus value)
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:12: warning: conversion of 'EStatus' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-enum-to-bool-conversion]
+ return static_cast<bool>(value);
+}
+
+enum EResult : short
+{
+ OK = 1,
+ NOT_OK
+};
+
+bool testEnumConversion(const EResult& value)
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:12: warning: conversion of 'EResult' into 'bool' will always return 'true', enum doesn't have a zero-value enumerator [bugprone-enum-to-bool-conversion]
+ return value;
+}
+
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle <bugprone/dangling-handle.html>`_,
`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers.html>`_,
`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters.html>`_,
+ `bugprone-enum-to-bool-conversion <bugprone/enum-to-bool-conversion.html>`_,
`bugprone-exception-escape <bugprone/exception-escape.html>`_,
`bugprone-fold-init-type <bugprone/fold-init-type.html>`_,
`bugprone-forward-declaration-namespace <bugprone/forward-declaration-namespace.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst
@@ -0,0 +1,58 @@
+.. title:: clang-tidy - bugprone-enum-to-bool-conversion
+
+bugprone-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.
+
+The check produces false positives if the ``enum`` is used to store other values
+(used as a bit-mask or zero-initialized on purpose). To deal with false-positives,
+``//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 (not status)
+ {
+ // this true-branch in theory wont 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.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,12 @@
This check relies heavily on, but is not exclusive to, the functions from
the *Annex K. "Bounds-checking interfaces"* of C11.
+- New :doc:`bugprone-enum-to-bool-conversion
+ <clang-tidy/checks/bugprone/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:`cppcoreguidelines-avoid-capture-default-when-capturing-this
<clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this>` check.
Index: clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h
@@ -0,0 +1,36 @@
+//===--- EnumToBoolConversionCheck.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_ENUMTOBOOLCONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ENUMTOBOOLCONVERSIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+/// Warns about `enum` to `bool` conversions where `enum` doesn't have a
+/// zero-value enumerator.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/enum-to-bool-conversion.html
+class EnumToBoolConversionCheck : public ClangTidyCheck {
+public:
+ EnumToBoolConversionCheck(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_ENUMTOBOOLCONVERSIONCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.cpp
@@ -0,0 +1,76 @@
+//===--- EnumToBoolConversionCheck.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 "EnumToBoolConversionCheck.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 auto *Definition = Node.getDefinition();
+ return Definition and Node.isComplete() and
+ std::none_of(
+ Definition->enumerator_begin(), Definition->enumerator_end(),
+ [](const auto *Value) { return Value->getInitVal().isZero(); });
+}
+
+} // namespace
+
+EnumToBoolConversionCheck::EnumToBoolConversionCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ EnumIgnoreList(
+ utils::options::parseStringList(Options.get("EnumIgnoreList", ""))) {}
+
+void EnumToBoolConversionCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "EnumIgnoreList",
+ utils::options::serializeStringList(EnumIgnoreList));
+}
+
+bool EnumToBoolConversionCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus;
+}
+
+void EnumToBoolConversionCheck::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 EnumToBoolConversionCheck::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
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -15,6 +15,7 @@
DanglingHandleCheck.cpp
DynamicStaticInitializersCheck.cpp
EasilySwappableParametersCheck.cpp
+ EnumToBoolConversionCheck.cpp
ExceptionEscapeCheck.cpp
FoldInitTypeCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -20,6 +20,7 @@
#include "DanglingHandleCheck.h"
#include "DynamicStaticInitializersCheck.h"
#include "EasilySwappableParametersCheck.h"
+#include "EnumToBoolConversionCheck.h"
#include "ExceptionEscapeCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
@@ -102,6 +103,8 @@
"bugprone-dynamic-static-initializers");
CheckFactories.registerCheck<EasilySwappableParametersCheck>(
"bugprone-easily-swappable-parameters");
+ CheckFactories.registerCheck<EnumToBoolConversionCheck>(
+ "bugprone-enum-to-bool-conversion");
CheckFactories.registerCheck<ExceptionEscapeCheck>(
"bugprone-exception-escape");
CheckFactories.registerCheck<FoldInitTypeCheck>("bugprone-fold-init-type");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
