Skip to content

[clang-tidy] Improve performance-enum-size to exclude empty enums #71640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

PiotrZSL
Copy link
Member

@PiotrZSL PiotrZSL commented Nov 8, 2023

Enums without enumerators (empty) are now excluded from analysis as it's not possible to peroperly determinate new narrowed type, and such enums can be used in diffrent way, like as strong-types.

Closes #71544

@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2023

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Enums without enumerators (empty) are now excluded from analysis as it's not possible to peroperly determinate new narrowed type, and such enums can be used in diffrent way, like as strong-types.

Closes #71544


Full diff: https://github.com/llvm/llvm-project/pull/71640.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp (+5)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst (+2)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp (+3)
diff --git a/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp b/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
index 0d44b8c7706c3c4..b1df9d74cf661ee 100644
--- a/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
@@ -23,6 +23,10 @@ namespace clang::tidy::performance {
 
 namespace {
 
+AST_MATCHER(EnumDecl, hasEnumerators) {
+  return Node.enumerator_begin() != Node.enumerator_end();
+}
+
 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();
@@ -93,6 +97,7 @@ bool EnumSizeCheck::isLanguageVersionSupported(
 void EnumSizeCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       enumDecl(unless(isExpansionInSystemHeader()), isDefinition(),
+               hasEnumerators(),
                unless(matchers::matchesAnyListedName(EnumIgnoreList)))
           .bind("e"),
       this);
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
index 08054123366eee4..f72b8c7eabc2221 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
@@ -58,6 +58,8 @@ terms of memory usage and cache performance. However, it's important to
 consider the trade-offs and potential impact on code readability and
 maintainability.
 
+Enums without enumerators (empty) are excluded from analysis.
+
 Requires C++11 or above.
 Does not provide auto-fixes.
 
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
index 37481a8141c5c45..782c12080f5180e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
@@ -102,4 +102,7 @@ enum class IgnoredSecondEnum
     unused2 = 2
 };
 
+enum class EnumClassWithoutValues : int {};
+enum EnumWithoutValues {};
+
 }

@PiotrZSL
Copy link
Member Author

PiotrZSL commented Nov 8, 2023

No release notes entry, as this check were added in this release.

Copy link
Contributor

@HerrCai0907 HerrCai0907 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

github-actions bot commented Nov 8, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Enums without enumerators (empty) are now excluded
from analysis as it's not possible to peroperly determinate
new narrowed type, and such enums can be used in diffrent
way, like as strong-types.
@PiotrZSL PiotrZSL force-pushed the 71544-fr-alter-clang-tidy-check-performance-enum-size-to-not-trigger-if-enum-class-is-empty branch from 5fca3fd to f745595 Compare November 8, 2023 09:48
@PiotrZSL PiotrZSL merged commit 3716b5b into llvm:main Nov 8, 2023
@PiotrZSL PiotrZSL deleted the 71544-fr-alter-clang-tidy-check-performance-enum-size-to-not-trigger-if-enum-class-is-empty branch November 8, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Alter clang-tidy check "performance-enum-size" to not trigger if enum class is empty
3 participants