[clang-tools-extra] [clang-tidy] only diagnose definitions in readability-enum-initial-value (PR #107652)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 16 14:11:22 PDT 2024


https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/107652

>From 991b6a70b81a9bdef6edf931d26e9430c56bf0ca Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sat, 7 Sep 2024 00:37:57 +0200
Subject: [PATCH 1/3] [clang-tidy] only diagnose definitions in
 readability-enum-initial-value

With the `isDefinition` matcher, the analysis and diagnostics will be
constrained to definitions only. Previously forward declarations were
diagnosed as well.

Fixes #107590
---
 .../readability/EnumInitialValueCheck.cpp          | 14 ++++++++------
 clang-tools-extra/docs/ReleaseNotes.rst            |  4 ++++
 .../checkers/readability/enum-initial-value.c      | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 8f2841c32259a2..60b129196ba955 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -141,16 +141,18 @@ void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 }
 
 void EnumInitialValueCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-      enumDecl(unless(isMacro()), unless(hasConsistentInitialValues()))
-          .bind("inconsistent"),
-      this);
+  Finder->addMatcher(enumDecl(isDefinition(), unless(isMacro()),
+                              unless(hasConsistentInitialValues()))
+                         .bind("inconsistent"),
+                     this);
   if (!AllowExplicitZeroFirstInitialValue)
     Finder->addMatcher(
-        enumDecl(hasZeroInitialValueForFirstEnumerator()).bind("zero_first"),
+        enumDecl(isDefinition(), hasZeroInitialValueForFirstEnumerator())
+            .bind("zero_first"),
         this);
   if (!AllowExplicitSequentialInitialValues)
-    Finder->addMatcher(enumDecl(unless(isMacro()), hasSequentialInitialValues())
+    Finder->addMatcher(enumDecl(isDefinition(), unless(isMacro()),
+                                hasSequentialInitialValues())
                            .bind("sequential"),
                        this);
 }
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8d0c093b312dd5..7c549309b5b3fc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,6 +132,10 @@ Changes in existing checks
   <clang-tidy/checks/modernize/use-std-print>` check to support replacing
   member function calls too.
 
+- Improved :doc:`readability-enum-initial-value
+  <clang-tidy/checks/readability-enum-initial-value>` check to only issue
+  diagnostics for the definition of an ``enum``.
+
 - Improved :doc:`performance-avoid-endl
   <clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as
   placeholder when lexer cannot get source text.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
index c66288cbe3e957..36727d00c10a48 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
@@ -78,3 +78,17 @@ enum EnumSequentialInitialValue {
   EnumSequentialInitialValue_2 = 4,
   // CHECK-FIXES-ENABLE: EnumSequentialInitialValue_2 ,
 };
+
+// gh107590
+enum WithFwdDecl : int;
+
+enum WithFwdDecl : int {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital values in enum 'WithFwdDecl' are not consistent
+  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: inital values in enum 'WithFwdDecl' are not consistent
+  E0,
+  // CHECK-FIXES: E0 = 0,
+  E1 = 1,
+  E2,
+  // CHECK-FIXES: E2 = 2,
+};
+

>From 48d3d21ea605ea5794630d7085d0d939564fc5fc Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sat, 7 Sep 2024 00:57:08 +0200
Subject: [PATCH 2/3] add fwd decl test for the other cases + fix diagnostic
 typo

---
 .../readability/EnumInitialValueCheck.cpp     |  2 +-
 .../checkers/readability/enum-initial-value.c | 55 +++++++++++++------
 .../readability/enum-initial-value.cpp        |  2 +-
 3 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
index 60b129196ba955..1cb95c2b2347b7 100644
--- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp
@@ -161,7 +161,7 @@ void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("inconsistent")) {
     DiagnosticBuilder Diag =
         diag(Enum->getBeginLoc(),
-             "inital values in enum %0 are not consistent, consider explicit "
+             "initial values in enum %0 are not consistent, consider explicit "
              "initialization of all, none or only the first enumerator")
         << Enum;
     for (const EnumConstantDecl *ECD : Enum->enumerators())
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
index 36727d00c10a48..b9a34d0683d7f3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c
@@ -6,8 +6,8 @@
 // RUN:     }}'
 
 enum EError {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital values in enum 'EError' are not consistent
-  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: inital values in enum 'EError' are not consistent
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EError' are not consistent
+  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum 'EError' are not consistent
   EError_a = 1,
   EError_b,
   // CHECK-FIXES: EError_b = 2,
@@ -34,8 +34,8 @@ enum EAll {
 
 #define ENUMERATOR_1 EMacro1_b
 enum EMacro1 {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital values in enum 'EMacro1' are not consistent
-  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: inital values in enum 'EMacro1' are not consistent
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EMacro1' are not consistent
+  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum 'EMacro1' are not consistent
   EMacro1_a = 1,
   ENUMERATOR_1,
   // CHECK-FIXES: ENUMERATOR_1 = 2,
@@ -45,8 +45,8 @@ enum EMacro1 {
 
 #define ENUMERATOR_2 EMacro2_b = 2
 enum EMacro2 {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital values in enum 'EMacro2' are not consistent
-  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: inital values in enum 'EMacro2' are not consistent
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'EMacro2' are not consistent
+  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum 'EMacro2' are not consistent
   EMacro2_a = 1,
   ENUMERATOR_2,
   EMacro2_c,
@@ -80,15 +80,38 @@ enum EnumSequentialInitialValue {
 };
 
 // gh107590
-enum WithFwdDecl : int;
-
-enum WithFwdDecl : int {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital values in enum 'WithFwdDecl' are not consistent
-  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: inital values in enum 'WithFwdDecl' are not consistent
-  E0,
-  // CHECK-FIXES: E0 = 0,
-  E1 = 1,
-  E2,
-  // CHECK-FIXES: E2 = 2,
+enum WithFwdDeclInconsistent : int;
+
+enum WithFwdDeclInconsistent : int {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: initial values in enum 'WithFwdDeclInconsistent' are not consistent
+  // CHECK-MESSAGES-ENABLE: :[[@LINE-2]]:1: warning: initial values in enum 'WithFwdDeclInconsistent' are not consistent
+  EFI0,
+  // CHECK-FIXES: EFI0 = 0,
+  EFI1 = 1,
+  EFI2,
+  // CHECK-FIXES: EFI2 = 2,
+};
+
+enum WithFwdDeclZeroFirst : int;
+
+enum WithFwdDeclZeroFirst : int {
+  // CHECK-MESSAGES-ENABLE: :[[@LINE+1]]:3: warning: zero initial value for the first enumerator in 'WithFwdDeclZeroFirst' can be disregarded
+  EFZ0 = 0,
+  // CHECK-FIXES-ENABLE: EFZ0 ,
+  EFZ1,
+  EFZ2,
+};
+
+
+enum WithFwdDeclSequential : int;
+
+enum WithFwdDeclSequential : int {
+  // CHECK-MESSAGES-ENABLE: :[[@LINE-1]]:1: warning: sequential initial value in 'WithFwdDeclSequential' can be ignored
+  EFS0 = 2,
+  // CHECK-FIXES-ENABLE: EFS0 = 2,
+  EFS1 = 3,
+  // CHECK-FIXES-ENABLE: EFS1 ,
+  EFS2 = 4,
+  // CHECK-FIXES-ENABLE: EFS2 ,
 };
 
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 3c4ba970372a07..9d324a39ee6a36 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
@@ -1,7 +1,7 @@
 // RUN: %check_clang_tidy %s readability-enum-initial-value %t
 
 enum class EError {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1:  warning: inital values in enum 'EError' are not consistent
+  // CHECK-MESSAGES: :[[@LINE-1]]:1:  warning: initial values in enum 'EError' are not consistent
   EError_a = 1,
   EError_b,
   // CHECK-FIXES: EError_b = 2,

>From e94f36ba14b1a070b9eb499eb0298657a6295c56 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sat, 7 Sep 2024 00:58:29 +0200
Subject: [PATCH 3/3] fix release note + mention diag message fix

---
 clang-tools-extra/docs/ReleaseNotes.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7c549309b5b3fc..1ebcc5e8d9c55a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,8 +133,9 @@ Changes in existing checks
   member function calls too.
 
 - Improved :doc:`readability-enum-initial-value
-  <clang-tidy/checks/readability-enum-initial-value>` check to only issue
-  diagnostics for the definition of an ``enum``.
+  <clang-tidy/checks/readability/enum-initial-value>` check by only issuing
+  diagnostics for the definition of an ``enum``, and by fixing a typo in the
+  diagnostic.
 
 - Improved :doc:`performance-avoid-endl
   <clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as



More information about the cfe-commits mailing list