[clang-tools-extra] 3397dae - [clang-tidy] Add performance-enum-size check
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 25 10:50:47 PDT 2023
Author: Piotr Zegar
Date: 2023-07-25T17:50:19Z
New Revision: 3397dae69e09d33301a7620171cabc301d1f3abb
URL: https://github.com/llvm/llvm-project/commit/3397dae69e09d33301a7620171cabc301d1f3abb
DIFF: https://github.com/llvm/llvm-project/commit/3397dae69e09d33301a7620171cabc301d1f3abb.diff
LOG: [clang-tidy] Add performance-enum-size check
Finds enum type definitions that could use smaller integral type as a base.
Reviewed By: xgupta
Differential Revision: https://reviews.llvm.org/D144135
Added:
clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h
clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
Modified:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index de13894dabc346..f4bcee7daf184d 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 00000000000000..0d44b8c7706c3c
--- /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 <algorithm>
+#include <cinttypes>
+#include <cstdint>
+#include <limits>
+#include <utility>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+
+namespace {
+
+const std::uint64_t Min8 =
+ std::imaxabs(std::numeric_limits<std::int8_t>::min());
+const std::uint64_t Max8 = std::numeric_limits<std::int8_t>::max();
+const std::uint64_t Min16 =
+ std::imaxabs(std::numeric_limits<std::int16_t>::min());
+const std::uint64_t Max16 = std::numeric_limits<std::int16_t>::max();
+const std::uint64_t Min32 =
+ std::imaxabs(std::numeric_limits<std::int32_t>::min());
+const std::uint64_t Max32 = std::numeric_limits<std::int32_t>::max();
+
+std::pair<const char *, std::uint32_t>
+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<std::uint8_t>::max()) {
+ return {"std::uint8_t", sizeof(std::uint8_t)};
+ }
+
+ if (Max <= std::numeric_limits<std::uint16_t>::max() &&
+ Size > sizeof(std::uint16_t)) {
+ return {"std::uint16_t", sizeof(std::uint16_t)};
+ }
+
+ if (Max <= std::numeric_limits<std::uint32_t>::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<EnumDecl>("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<std::uint64_t>(MaxV, InitVal.getZExtValue());
+ } else {
+ MinV = std::max<std::uint64_t>(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 00000000000000..4d797602ede8b9
--- /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 <vector>
+
+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<StringRef> 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 54bf14fae8ca79..9e0fa6f88b36a0 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<AvoidEndlCheck>("performance-avoid-endl");
+ CheckFactories.registerCheck<EnumSizeCheck>("performance-enum-size");
CheckFactories.registerCheck<FasterStringFindCheck>(
"performance-faster-string-find");
CheckFactories.registerCheck<ForRangeCopyCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e602fceda6b1f7..ef6a8ce2788823 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
+ <clang-tidy/checks/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 af147db71eb726..1804346da020e2 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/exception-escape.html>`_,
`openmp-use-default-none <openmp/use-default-none.html>`_,
`performance-avoid-endl <performance/avoid-endl.html>`_, "Yes"
+ `performance-enum-size <performance/enum-size.html>`_,
`performance-faster-string-find <performance/faster-string-find.html>`_, "Yes"
`performance-for-range-copy <performance/for-range-copy.html>`_, "Yes"
`performance-implicit-conversion-in-loop <performance/implicit-conversion-in-loop.html>`_,
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 00000000000000..08054123366eee
--- /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> 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<Color>`` 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> 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<Color>`` 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 00000000000000..3ebef396096e20
--- /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
+};
+
+}
More information about the cfe-commits
mailing list