[clang-tools-extra] [clang-tidy] readability-enum-initial-value: fix self-reference detection (PR #228401)

[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: fix self-reference detection

Fix `AllowReferencedInitialValues` failing to recognize references to another enumerator in unscoped enums when the reference is implicitly converted to the enum's underlying type.

Add a regression test for this case.

---
LLVM already supports enumerators that explicitly refer to another enumerator through AllowReferencedInitialValues.
For example, the existing implementation supports:

  E_a,
  E_b,
  E_last = E_b,
};
The existing upstream tests also cover cases where the first enumerator is explicitly initialized and a later enumerator refers to an earlier one.

// OK: first-only + self-ref.
enum ERef2 {
  ERef2_a = 0,
  ERef2_b,
  ERef2_last = ERef2_b,
};
However, the following valid pattern was not handled correctly:

enum EFirstThenRefs {
  EFirstThenRefs_a = 0,
  EFirstThenRefs_b,
  EFirstThenRefs_c,
  EFirstThenRefs_first = EFirstThenRefs_a,
  EFirstThenRefs_last = EFirstThenRefs_c,
};
Here, the enum has an explicit first value, followed by implicitly initialized enumerators, and then two enumerators that explicitly reference earlier enumerators.
With AllowReferencedInitialValues enabled, these references should be treated as allowed references rather than as inconsistent explicit initializers.
This PR fixes the existing reference handling so that this pattern is recognized correctly.
The change does not introduce a new reference/alias option; it fixes the existing AllowReferencedInitialValues behavior for this case.

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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp (+12-4) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.md (+3-2) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-refs.cpp (+19) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 620c5bece4ebad..b95c7daa933a8b 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -21,11 +21,19 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::readability {
 
 /// Check if \p ECD is initialized by referencing another enumerator in the
-/// same enum (e.g., `last = first`).
+/// same enum (e.g., `last = first`). For unscoped enums, the reference may be
+/// wrapped in an implicit conversion to the enum's underlying type.
 static bool isSelfReference(const EnumConstantDecl *ECD) {
-  const auto *CE = dyn_cast_if_present<ConstantExpr>(ECD->getInitExpr());
-  const auto *DRE =
-      dyn_cast_if_present<DeclRefExpr>(CE ? CE->getSubExpr() : nullptr);
+  const Expr *Init = ECD->getInitExpr();
+  if (!Init)
+    return false;
+
+  if (const auto *CE = dyn_cast<ConstantExpr>(Init))
+    Init = CE->getSubExpr();
+
+  Init = Init->IgnoreImpCasts();
+
+  const auto *DRE = dyn_cast<DeclRefExpr>(Init);
   const auto *RefECD =
       dyn_cast_if_present<EnumConstantDecl>(DRE ? DRE->getDecl() : nullptr);
   return RefECD && RefECD->getDeclContext() == ECD->getDeclContext();
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..d504e4bb86b4d0 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
@@ -94,8 +94,9 @@ enum G {
 ````{option} AllowReferencedInitialValues
 When `true`, enumerators initialized by referencing another enumerator
 in the same enum are allowed, and the remaining enumerators are checked for
-consistency. This implements the `INT09-C-EX1` exception from the CERT C
-Coding Standard.
+consistency. References to earlier enumerators are also recognized when they
+occur after implicitly initialized enumerators. This implements the
+`INT09-C-EX1` exception from the CERT C Coding Standard.
 Default is `false`.
 
 ```c++
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-refs.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-refs.cpp
index f66b75fec37100..5f1b0e282816e0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-refs.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value-allow-refs.cpp
@@ -24,6 +24,25 @@ enum class ERefAll {
   ERefAll_last = ERefAll_b,
 };
 
+// OK: first-only + self-refs, where later self-references do not need to
+// immediately follow the enumerator they reference and do not require every
+// preceding enumerator to be explicitly initialized.
+enum EFirstThenRefs {
+  EFirstThenRefs_a = 0,
+  EFirstThenRefs_b,
+  EFirstThenRefs_c,
+  EFirstThenRefs_first = EFirstThenRefs_a,
+  EFirstThenRefs_last = EFirstThenRefs_c,
+};
+
+// OK: a self-reference may appear before later implicit enumerators.
+enum ERefThenImplicit {
+  ERefThenImplicit_a = 0,
+  ERefThenImplicit_alias = ERefThenImplicit_a,
+  ERefThenImplicit_b,
+  ERefThenImplicit_c,
+};
+
 // Error: literal duplicate (not a reference).
 enum class ERefErr {
   // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'ERefErr' are not consistent

``````````

</details>


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