Skip to content

[clang-tidy] only diagnose definitions in readability-enum-initial-value #107652

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

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

5chmidti
Copy link
Contributor

@5chmidti 5chmidti commented Sep 6, 2024

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

Fixes #107590

@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2024

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Julian Schmidt (5chmidti)

Changes

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

Fixes #107590


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp (+8-6)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c (+14)
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 8d028f8863cb7a..469dd4e54b1181 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,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:`readablility-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check
   by adding the option `UseUpperCaseLiteralSuffix` to select the
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,
+};
+

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

Fixes llvm#107590
@5chmidti 5chmidti force-pushed the clang_tidy_enum_initial_only_def branch from c3329bc to e94f36b Compare September 16, 2024 21:11
@5chmidti 5chmidti merged commit caaac84 into llvm:main Sep 17, 2024
9 checks passed
@5chmidti 5chmidti deleted the clang_tidy_enum_initial_only_def branch September 17, 2024 08:44
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
…lue (llvm#107652)

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

Fixes llvm#107590
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.

[clang-tidy] Duplicate readability-enum-initial-value for forward-declared enum
4 participants