diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/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( "bugprone-easily-swappable-parameters"); + CheckFactories.registerCheck( + "bugprone-enum-to-bool-conversion"); CheckFactories.registerCheck( "bugprone-exception-escape"); CheckFactories.registerCheck("bugprone-fold-init-type"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h b/clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.h @@ -0,0 +1,36 @@ +//===--- EnumToBoolConversionCheck.h - clang-tidy ---------------*- C++ -*-===// +// +// © 2023 Nokia +// 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 + +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 diff --git a/clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/EnumToBoolConversionCheck.cpp @@ -0,0 +1,68 @@ +//===--- EnumToBoolConversionCheck.cpp - clang-tidy -----------------------===// +// +// © 2023 Nokia +// 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 + +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("cast"); + const auto *Enum = Result.Nodes.getNodeAs("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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/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 + ` 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 ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/enum-to-bool-conversion.rst new file mode 100644 --- /dev/null +++ b/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. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -86,6 +86,7 @@ `bugprone-dangling-handle `_, `bugprone-dynamic-static-initializers `_, `bugprone-easily-swappable-parameters `_, + `bugprone-enum-to-bool-conversion `_, `bugprone-exception-escape `_, `bugprone-fold-init-type `_, `bugprone-forward-declaration-namespace `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion-cpp11.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion-cpp11.cpp new file mode 100644 --- /dev/null +++ b/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(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; +} + +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-bool-conversion.cpp new file mode 100644 --- /dev/null +++ b/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(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; +} + +}