ClockMan created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan updated this revision to Diff 497408.
ClockMan added a comment.
Eugene.Zelenko added reviewers: aaron.ballman, carlosgalvezp.
ClockMan updated this revision to Diff 497692.
ClockMan published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Removed "Offers fixes" from list.rst
ClockMan added a comment.
Correct copyrights and commit author
ClockMan added a comment.
Ready for review
================
Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h:28
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
----------------
Should language be checked for C++?
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst:6
+
+Detect implicit and explicit casts of `enum` type into `bool` where `enum` type
+doesn't have a zero-value enumerator. If `enum` is used only to hold values
----------------
Please synchronize first statement with statement in Release Notes. Please use
double back-ticks for language constructs.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst:49
+
+ Default value: '^$'.
+ Regexp used to ignore usages of enum declarations that match regexp.
----------------
Please use single back-ticks for option values. Default value is usually placed
after option description.
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.
Repository:
rG LLVM Github Monorepo
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,119 @@
+// 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.EnumIgnoreRegexp, value: '^::without::issue::IgnoredEnum$'}]}"
+
+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
+};
+
+bool testIgnored(IgnoredEnum 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,50 @@
+.. 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 `enum` is used only to hold values
+equal to its enumerators, then conversion to `bool` will always result in `true`
+value.
+
+Check will produce false-positives if `enum` will be used to store other values
+(used as bit-mask or zero-initialized on purpose).
+
+`//NOLINT` or casting first to underlying type before casting to `bool` can be
+used to deal with false-positives.
+
+Check won't produce warnings if definition of `enum` is not available.
+C++11 enum classes are supported.
+
+Example
+-------
+
+.. code-block:: c++
+
+ enum EStatus
+ {
+ OK = 1,
+ NOT_OK,
+ UNKNOWN
+ };
+
+ void process(EStatus status)
+ {
+ if (not status)
+ {
+ // log error <- this true-branch in theory wont be executed
+ return;
+ }
+
+ // proceed with "valid data"
+ }
+
+
+Options
+-------
+
+.. option:: EnumIgnoreRegexp
+
+ Default value: '^$'.
+ Regexp used to ignore usages of enum declarations that match regexp.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -105,6 +105,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.
+
+ Warns about `enum` to `bool` conversions where `enum` 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,35 @@
+//===--- 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 <string>
+
+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;
+
+private:
+ const std::string EnumIgnoreRegexp;
+};
+
+} // 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,67 @@
+//===--- 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 "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().isNullValue();
+ });
+}
+
+} // namespace
+
+EnumToBoolConversionCheck::EnumToBoolConversionCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ EnumIgnoreRegexp(Options.get("EnumIgnoreRegexp", "^$")) {}
+
+void EnumToBoolConversionCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "EnumIgnoreRegexp", EnumIgnoreRegexp);
+}
+
+void EnumToBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ castExpr(hasCastKind(CK_IntegralToBoolean),
+ unless(isExpansionInSystemHeader()), hasType(booleanType()),
+ hasSourceExpression(
+ expr(hasType(qualType(hasCanonicalType(hasDeclaration(
+ enumDecl(isCompleteAndHasNoZeroValue(),
+ unless(matchesName(EnumIgnoreRegexp)))
+ .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
