diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index de13894dabc34..f4bcee7daf184 100644 --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyPerformanceModule AvoidEndlCheck.cpp + EnumSizeCheck.cpp FasterStringFindCheck.cpp ForRangeCopyCheck.cpp ImplicitConversionInLoopCheck.cpp diff --git a/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp b/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp new file mode 100644 index 0000000000000..0d44b8c7706c3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp @@ -0,0 +1,135 @@ +//===--- EnumSizeCheck.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 "EnumSizeCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include +#include +#include +#include +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +namespace { + +const std::uint64_t Min8 = + std::imaxabs(std::numeric_limits::min()); +const std::uint64_t Max8 = std::numeric_limits::max(); +const std::uint64_t Min16 = + std::imaxabs(std::numeric_limits::min()); +const std::uint64_t Max16 = std::numeric_limits::max(); +const std::uint64_t Min32 = + std::imaxabs(std::numeric_limits::min()); +const std::uint64_t Max32 = std::numeric_limits::max(); + +std::pair +getNewType(std::size_t Size, std::uint64_t Min, std::uint64_t Max) noexcept { + if (Min) { + if (Min <= Min8 && Max <= Max8) { + return {"std::int8_t", sizeof(std::int8_t)}; + } + + if (Min <= Min16 && Max <= Max16 && Size > sizeof(std::int16_t)) { + return {"std::int16_t", sizeof(std::int16_t)}; + } + + if (Min <= Min32 && Max <= Max32 && Size > sizeof(std::int32_t)) { + return {"std::int32_t", sizeof(std::int32_t)}; + } + + return {}; + } + + if (Max) { + if (Max <= std::numeric_limits::max()) { + return {"std::uint8_t", sizeof(std::uint8_t)}; + } + + if (Max <= std::numeric_limits::max() && + Size > sizeof(std::uint16_t)) { + return {"std::uint16_t", sizeof(std::uint16_t)}; + } + + if (Max <= std::numeric_limits::max() && + Size > sizeof(std::uint32_t)) { + return {"std::uint32_t", sizeof(std::uint32_t)}; + } + + return {}; + } + + // Zero case + return {"std::uint8_t", sizeof(std::uint8_t)}; +} + +} // namespace + +EnumSizeCheck::EnumSizeCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + EnumIgnoreList( + utils::options::parseStringList(Options.get("EnumIgnoreList", ""))) {} + +void EnumSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "EnumIgnoreList", + utils::options::serializeStringList(EnumIgnoreList)); +} + +bool EnumSizeCheck::isLanguageVersionSupported( + const LangOptions &LangOpts) const { + return LangOpts.CPlusPlus11; +} + +void EnumSizeCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + enumDecl(unless(isExpansionInSystemHeader()), isDefinition(), + unless(matchers::matchesAnyListedName(EnumIgnoreList))) + .bind("e"), + this); +} + +void EnumSizeCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs("e"); + const QualType BaseType = MatchedDecl->getIntegerType().getCanonicalType(); + if (!BaseType->isIntegerType()) + return; + + const std::uint32_t Size = Result.Context->getTypeSize(BaseType) / 8U; + if (1U == Size) + return; + + std::uint64_t MinV = 0U; + std::uint64_t MaxV = 0U; + + for (const auto &It : MatchedDecl->enumerators()) { + const llvm::APSInt &InitVal = It->getInitVal(); + if ((InitVal.isUnsigned() || InitVal.isNonNegative())) { + MaxV = std::max(MaxV, InitVal.getZExtValue()); + } else { + MinV = std::max(MinV, InitVal.abs().getZExtValue()); + } + } + + auto NewType = getNewType(Size, MinV, MaxV); + if (!NewType.first || Size <= NewType.second) + return; + + diag(MatchedDecl->getLocation(), + "enum %0 uses a larger base type (%1, size: %2 %select{byte|bytes}5) " + "than necessary for its value set, consider using '%3' (%4 " + "%select{byte|bytes}6) as the base type to reduce its size") + << MatchedDecl << MatchedDecl->getIntegerType() << Size << NewType.first + << NewType.second << (Size > 1U) << (NewType.second > 1U); +} + +} // namespace clang::tidy::performance diff --git a/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h b/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h new file mode 100644 index 0000000000000..4d797602ede8b --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h @@ -0,0 +1,36 @@ +//===--- EnumSizeCheck.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_PERFORMANCE_ENUMSIZECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_ENUMSIZECHECK_H + +#include "../ClangTidyCheck.h" +#include + +namespace clang::tidy::performance { + +/// Finds `enum` type definitions that could use smaller integral type as a +/// base. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/performance/enum-size.html +class EnumSizeCheck : public ClangTidyCheck { +public: + EnumSizeCheck(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 EnumIgnoreList; +}; + +} // namespace clang::tidy::performance + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_ENUMSIZECHECK_H diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp index 54bf14fae8ca7..9e0fa6f88b36a 100644 --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "AvoidEndlCheck.h" +#include "EnumSizeCheck.h" #include "FasterStringFindCheck.h" #include "ForRangeCopyCheck.h" #include "ImplicitConversionInLoopCheck.h" @@ -35,6 +36,7 @@ class PerformanceModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck("performance-avoid-endl"); + CheckFactories.registerCheck("performance-enum-size"); CheckFactories.registerCheck( "performance-faster-string-find"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e602fceda6b1f..ef6a8ce278882 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -97,6 +97,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`performance-enum-size + ` check. + + Recommends the smallest possible underlying type for an ``enum`` or ``enum`` + class based on the range of its enumerators. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index af147db71eb72..1804346da020e 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -323,6 +323,7 @@ Clang-Tidy Checks `openmp-exception-escape `_, `openmp-use-default-none `_, `performance-avoid-endl `_, "Yes" + `performance-enum-size `_, `performance-faster-string-find `_, "Yes" `performance-for-range-copy `_, "Yes" `performance-implicit-conversion-in-loop `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst new file mode 100644 index 0000000000000..08054123366ee --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst @@ -0,0 +1,72 @@ +.. title:: clang-tidy - performance-enum-size + +performance-enum-size +===================== + +Recommends the smallest possible underlying type for an ``enum`` or ``enum`` +class based on the range of its enumerators. Analyzes the values of the +enumerators in an ``enum`` or ``enum`` class, including signed values, to +recommend the smallest possible underlying type that can represent all the +values of the ``enum``. The suggested underlying types are the integral types +``std::uint8_t``, ``std::uint16_t``, and ``std::uint32_t`` for unsigned types, +and ``std::int8_t``, ``std::int16_t``, and ``std::int32_t`` for signed types. +Using the suggested underlying types can help reduce the memory footprint of +the program and improve performance in some cases. + +For example: + +.. code-block:: c++ + + // BEFORE + enum Color { + RED = -1, + GREEN = 0, + BLUE = 1 + }; + + std::optional color_opt; + +The `Color` ``enum`` uses the default underlying type, which is ``int`` in this +case, and its enumerators have values of -1, 0, and 1. Additionally, the +``std::optional`` object uses 8 bytes due to padding (platform +dependent). + +.. code-block:: c++ + + // AFTER + enum Color : std:int8_t { + RED = -1, + GREEN = 0, + BLUE = 1 + } + + std::optional color_opt; + + +In the revised version of the `Color` ``enum``, the underlying type has been +changed to ``std::int8_t``. The enumerator `RED` has a value of -1, which can +be represented by a signed 8-bit integer. + +By using a smaller underlying type, the memory footprint of the `Color` +``enum`` is reduced from 4 bytes to 1 byte. The revised version of the +``std::optional`` object would only require 2 bytes (due to lack of +padding), since it contains a single byte for the `Color` ``enum`` and a single +byte for the ``bool`` flag that indicates whether the optional value is set. + +Reducing the memory footprint of an ``enum`` can have significant benefits in +terms of memory usage and cache performance. However, it's important to +consider the trade-offs and potential impact on code readability and +maintainability. + +Requires C++11 or above. +Does not provide auto-fixes. + +Options +------- + +.. option:: EnumIgnoreList + + Option is used to ignore certain enum types. 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/test/clang-tidy/checkers/performance/enum-size.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp new file mode 100644 index 0000000000000..3ebef396096e2 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp @@ -0,0 +1,105 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s performance-enum-size %t -- \ +// RUN: -config="{CheckOptions: [{key: performance-enum-size.EnumIgnoreList, value: '::IgnoredEnum;IgnoredSecondEnum'}]}" + +namespace std +{ +using uint8_t = unsigned char; +using int8_t = signed char; +using uint16_t = unsigned short; +using int16_t = signed short; +using uint32_t = unsigned int; +using int32_t = signed int; +} + +enum class Value +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: enum 'Value' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size] +{ + supported +}; + + +enum class EnumClass : std::int16_t +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: enum 'EnumClass' uses a larger base type ('std::int16_t' (aka 'short'), size: 2 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size] +{ + supported +}; + +enum EnumWithType : std::uint16_t +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumWithType' uses a larger base type ('std::uint16_t' (aka 'unsigned short'), size: 2 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size] +{ + supported, + supported2 +}; + +enum EnumWithNegative +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumWithNegative' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size] +{ + s1 = -128, + s2 = -100, + s3 = 100, + s4 = 127 +}; + +enum EnumThatCanBeReducedTo2Bytes +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumThatCanBeReducedTo2Bytes' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int16_t' (2 bytes) as the base type to reduce its size [performance-enum-size] +{ + a1 = -128, + a2 = 0x6EEE +}; + +enum EnumOnlyNegative +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumOnlyNegative' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size] +{ + b1 = -125, + b2 = -50, + b3 = -10 +}; + +enum CorrectU8 : std::uint8_t +{ + c01 = 10, + c02 = 11 +}; + +enum CorrectU16 : std::uint16_t +{ + c11 = 10, + c12 = 0xFFFF +}; + +constexpr int getValue() +{ + return 256; +} + + +enum CalculatedDueToUnknown1 : unsigned int +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'CalculatedDueToUnknown1' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint16_t' (2 bytes) as the base type to reduce its size [performance-enum-size] +{ + c21 = 10, + c22 = getValue() +}; + +enum CalculatedDueToUnknown2 : unsigned int +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'CalculatedDueToUnknown2' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint16_t' (2 bytes) as the base type to reduce its size [performance-enum-size] +{ + c31 = 10, + c32 = c31 + 246 +}; + +enum class IgnoredEnum : std::uint32_t +{ + unused1 = 1, + unused2 = 2 +}; + +namespace internal +{ + +enum class IgnoredSecondEnum +{ + unused1 = 1, + unused2 = 2 +}; + +}