[clang-tools-extra] [clang-tidy] readability-enum-initial-value: add AllowConsecutiveInitialValuesExceptLast (PR #228399)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: garima-gupta511

<details>
<summary>Changes</summary>

[clang-tidy] readability-enum-initial-value: add AllowConsecutiveInitialValuesExceptLast

Add an opt-in option to allow enums where all enumerators except the last have explicit consecutive integer-literal initializers and the last enumerator is implicitly initialized.

For example:

enum E {
A = 10,
B = 11,
C = 12,
Size,
};

This pattern is commonly used when the final enumerator represents a count or sentinel value.

The new option is disabled by default so that the existing diagnostic behavior is unchanged unless the option is explicitly enabled.

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


6 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp (+45-7) 
- (modified) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h (+1) 
- (modified) clang-tools-extra/docs/ReleaseNotes.md (+8) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.md (+17) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-consecutive.cpp (+73) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp (+10) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 620c5bece4ebad..e42429e818de84 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
 
 using namespace clang::ast_matchers;
 
@@ -74,6 +75,34 @@ static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
   return Init->isIntegerConstantExpr(Enumerator->getASTContext());
 }
 
+/// Returns true if all but the last enumerator are explicitly initialized
+/// with consecutive (potentially negated) integer literal values and the
+/// last enumerator's value is left implicit, e.g.
+///   enum E { A = 10, B = 11, C = 12, Size };
+static bool hasConsecutiveInitialValuesExceptLast(const EnumDecl &Node) {
+  const llvm::SmallVector<const EnumConstantDecl *> Enumerators(
+      Node.enumerator_begin(), Node.enumerator_end());
+  if (Enumerators.size() < 2)
+    return false;
+
+  const EnumConstantDecl *const LastEnumerator = Enumerators.back();
+  if (LastEnumerator->getInitExpr() != nullptr)
+    return false;
+
+  const EnumConstantDecl *const FirstEnumerator = Enumerators.front();
+  if (!isInitializedByLiteral(FirstEnumerator))
+    return false;
+
+  llvm::APSInt PrevValue = FirstEnumerator->getInitVal();
+  for (size_t I = 1, E = Enumerators.size() - 1; I < E; ++I) {
+    const EnumConstantDecl *const Enumerator = Enumerators[I];
+    if (!isInitializedByLiteral(Enumerator) ||
+        Enumerator->getInitVal() != ++PrevValue)
+      return false;
+  }
+  return LastEnumerator->getInitVal() == ++PrevValue;
+}
+
 static void cleanInitialValue(const DiagnosticBuilder &Diag,
                               const EnumConstantDecl *ECD,
                               const SourceManager &SM,
@@ -101,10 +130,13 @@ AST_MATCHER(EnumDecl, isMacro) {
   return Loc.isMacroID();
 }
 
-AST_MATCHER_P(EnumDecl, hasConsistentInitialValues, bool, AllowSelfRefs) {
+AST_MATCHER_P2(EnumDecl, hasConsistentInitialValues, bool, AllowSelfRefs, bool,
+               AllowConsecutiveExceptLast) {
   return isNoneEnumeratorsInitialized(Node, AllowSelfRefs) ||
          isOnlyFirstEnumeratorInitialized(Node, AllowSelfRefs) ||
-         areAllEnumeratorsInitialized(Node);
+         areAllEnumeratorsInitialized(Node) ||
+         (AllowConsecutiveExceptLast &&
+          hasConsecutiveInitialValuesExceptLast(Node));
 }
 
 AST_MATCHER_P(EnumDecl, hasZeroInitialValueForFirstEnumerator, bool,
@@ -164,7 +196,9 @@ EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
       AllowExplicitSequentialInitialValues(
           Options.get("AllowExplicitSequentialInitialValues", true)),
       AllowReferencedInitialValues(
-          Options.get("AllowReferencedInitialValues", false)) {}
+          Options.get("AllowReferencedInitialValues", false)),
+      AllowConsecutiveInitialValuesExceptLast(
+          Options.get("AllowConsecutiveInitialValuesExceptLast", false)) {}
 
 void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AllowExplicitZeroFirstInitialValue",
@@ -173,14 +207,18 @@ void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
                 AllowExplicitSequentialInitialValues);
   Options.store(Opts, "AllowReferencedInitialValues",
                 AllowReferencedInitialValues);
+  Options.store(Opts, "AllowConsecutiveInitialValuesExceptLast",
+                AllowConsecutiveInitialValuesExceptLast);
 }
 
 void EnumInitialValueCheck::registerMatchers(MatchFinder *Finder) {
   const bool AllowSelfRefs = AllowReferencedInitialValues;
-  Finder->addMatcher(enumDecl(isDefinition(), unless(isMacro()),
-                              unless(hasConsistentInitialValues(AllowSelfRefs)))
-                         .bind("inconsistent"),
-                     this);
+  Finder->addMatcher(
+      enumDecl(isDefinition(), unless(isMacro()),
+               unless(hasConsistentInitialValues(
+                   AllowSelfRefs, AllowConsecutiveInitialValuesExceptLast)))
+          .bind("inconsistent"),
+      this);
   if (!AllowExplicitZeroFirstInitialValue)
     Finder->addMatcher(
         enumDecl(isDefinition(),
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h
index 155818869d28b3..65f3406db0fee4 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h
@@ -32,6 +32,7 @@ class EnumInitialValueCheck : public ClangTidyCheck {
   const bool AllowExplicitZeroFirstInitialValue;
   const bool AllowExplicitSequentialInitialValues;
   const bool AllowReferencedInitialValues;
+  const bool AllowConsecutiveInitialValuesExceptLast;
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index c2ee1b06c43e06..704f17b4ad5e20 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -272,6 +272,14 @@ infrastructure are described first, followed by tool-specific sections.
   `INT09-C-EX1` exception, allowing enumerators initialized by referencing
   another enumerator in the same enum (e.g., `last = first`).
 
+- Improved {doc}`readability-enum-initial-value
+  <clang-tidy/checks/readability/enum-initial-value>` check by adding
+  the {option}`AllowConsecutiveInitialValuesExceptLast` option, allowing
+  enums where all but the last enumerator are explicitly initialized with
+  consecutive integer literal values and the last enumerator's value is
+  left implicit (e.g., a trailing count/sentinel enumerator). This option
+  defaults to `false`, so the check's default behavior is unchanged.
+
 - Improved {doc}`readability-function-cognitive-complexity
   <clang-tidy/checks/readability/function-cognitive-complexity>` check by fixing
   a crash when checking a function declared with the `alias` attribute.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.md b/clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.md
index aec3af8c11eec2..de1db9a8991e0f 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.md
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.md
@@ -106,3 +106,20 @@ enum H {
 };
 ```
 ````
+
+````{option} AllowConsecutiveInitialValuesExceptLast
+When `true`, enums where all but the last enumerator are explicitly
+initialized with consecutive integer literal values, and the last
+enumerator is left uninitialized, are allowed. This is a common pattern
+for enumerations that use the last enumerator as a count/sentinel value.
+Default is `false`.
+
+```c++
+enum I {
+  i0 = 0,
+  i1 = 1,
+  i2 = 2,
+  i_size, // Allowed if AllowConsecutiveInitialValuesExceptLast is true.
+};
+```
+````
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-consecutive.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-consecutive.cpp
new file mode 100644
index 00000000000000..d739ab30fed54b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-consecutive.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s readability-enum-initial-value %t -- \
+// RUN:     -config='{CheckOptions: { \
+// RUN:         readability-enum-initial-value.AllowConsecutiveInitialValuesExceptLast: true, \
+// RUN:     }}'
+
+// OK: consecutive except last, starting at zero.
+enum class EConsecutive {
+  EConsecutive_a = 0,
+  EConsecutive_b = 1,
+  EConsecutive_c = 2,
+  EConsecutive_size,
+};
+
+// OK: consecutive except last, with a non-zero starting offset.
+enum class EConsecutiveOffset {
+  EConsecutiveOffset_a = 10,
+  EConsecutiveOffset_b = 11,
+  EConsecutiveOffset_c = 12,
+  EConsecutiveOffset_size,
+};
+
+// OK: consecutive except last, with negative values.
+enum class EConsecutiveNegative {
+  EConsecutiveNegative_a = -2,
+  EConsecutiveNegative_b = -1,
+  EConsecutiveNegative_c = 0,
+  EConsecutiveNegative_size,
+};
+
+// OK: minimal case, only two enumerators.
+enum class EConsecutiveMinimal {
+  EConsecutiveMinimal_a = 5,
+  EConsecutiveMinimal_size,
+};
+
+// Error: not consecutive (gap between b and c).
+enum class EConsecutiveBreak {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EConsecutiveBreak' are not consistent
+  EConsecutiveBreak_a = 0,
+  EConsecutiveBreak_b = 1,
+  EConsecutiveBreak_c = 3,
+  EConsecutiveBreak_size,
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: uninitialized enumerator 'EConsecutiveBreak_size' defined here
+  // CHECK-FIXES: EConsecutiveBreak_size = 4,
+};
+
+// Error: one of the "except last" enumerators is left uninitialized, so the
+// remaining explicit values are not consecutive with each other.
+enum class EConsecutiveGap {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EConsecutiveGap' are not consistent
+  EConsecutiveGap_a = 0,
+  EConsecutiveGap_b,
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: uninitialized enumerator 'EConsecutiveGap_b' defined here
+  // CHECK-FIXES: EConsecutiveGap_b = 1,
+  EConsecutiveGap_c = 2,
+  EConsecutiveGap_size,
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: uninitialized enumerator 'EConsecutiveGap_size' defined here
+  // CHECK-FIXES: EConsecutiveGap_size = 3,
+};
+
+// Error: the last enumerator is also explicitly initialized (the
+// "consecutive except last" pattern requires the last enumerator to be
+// implicit), and not every other enumerator is explicit either, so this
+// matches none of the accepted styles.
+enum class EConsecutiveLastMismatch {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EConsecutiveLastMismatch' are not consistent
+  EConsecutiveLastMismatch_a = 0,
+  EConsecutiveLastMismatch_b = 1,
+  EConsecutiveLastMismatch_c,
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: uninitialized enumerator 'EConsecutiveLastMismatch_c' defined here
+  // CHECK-FIXES: EConsecutiveLastMismatch_c = 2,
+  EConsecutiveLastMismatch_last = 10,
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp
index abbd33a1dd001c..0f4eac71a7705a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp
@@ -37,3 +37,13 @@ enum class ERef {
   // CHECK-FIXES: ERef_b = 1,
   ERef_last = ERef_b,
 };
+
+enum class EConsecutiveExceptLast {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EConsecutiveExceptLast' are not consistent
+  EConsecutiveExceptLast_a = 10,
+  EConsecutiveExceptLast_b = 11,
+  EConsecutiveExceptLast_c = 12,
+  EConsecutiveExceptLast_size,
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: uninitialized enumerator 'EConsecutiveExceptLast_size' defined here
+  // CHECK-FIXES: EConsecutiveExceptLast_size = 13,
+};

``````````

</details>


https://github.com/llvm/llvm-project/pull/228399