Skip to content

Conversation

felix642
Copy link
Contributor

Updated the check to ignore point static data members with in class initializer since removing the inline specifier would generate a compilation error

Fixes #80684

…a members

Updated the check to ignore point static data members with
in class initializer since removing the inline specifier would generate
a compilation error

Fixes llvm#80684
@felix642 felix642 force-pushed the redundant-inline-specifier-cinit-member-class branch from 45c89e2 to 0fa56a4 Compare February 11, 2024 17:49
@llvmbot
Copy link
Member

llvmbot commented Feb 11, 2024

@llvm/pr-subscribers-clang-tidy

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

Author: Félix-Antoine Constantin (felix642)

Changes

Updated the check to ignore point static data members with in class initializer since removing the inline specifier would generate a compilation error

Fixes #80684


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp (+11-6)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp (+14)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 0e8d17d4442478..3d6b052686c20c 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage,
     return VD->isInAnonymousNamespace();
   llvm_unreachable("Not a valid polymorphic type");
 }
+
+AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); }
+
 } // namespace
 
 static SourceLocation getInlineTokenLocation(SourceRange RangeLocation,
@@ -88,12 +91,14 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
         this);
 
   if (getLangOpts().CPlusPlus17) {
-    Finder->addMatcher(
-        varDecl(isInlineSpecified(),
-                anyOf(isInternalLinkage(StrictMode),
-                      allOf(isConstexpr(), hasAncestor(recordDecl()))))
-            .bind("var_decl"),
-        this);
+    const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
+    Finder->addMatcher(varDecl(isInlineSpecified(),
+                               anyOf(allOf(isInternalLinkage(StrictMode),
+                                           unless(allOf(hasInitialization(),
+                                                        IsPartOfRecordDecl))),
+                                     allOf(isConstexpr(), IsPartOfRecordDecl)))
+                           .bind("var_decl"),
+                       this);
   }
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ee68c8f49b3df2..303bfef17015a0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -160,6 +160,10 @@ Changes in existing checks
   `AllowStringArrays` option, enabling the exclusion of array types with deduced
   length initialized from string literals.
 
+- Improved :doc:`readability-redundant-inline-specifier
+  <clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
+  emit warnings for static data member with an in-class initializer.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
index cdd98d8fdc20f5..14f9e88f7e7218 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -135,3 +135,17 @@ INLINE_MACRO()
 
 #define INLINE_KW inline
 INLINE_KW void fn10() { }
+
+namespace {
+class A
+{
+public:
+  static inline float test = 3.0F;
+  static inline double test2 = 3.0;
+  static inline int test3 = 3;
+
+  static inline float test4;
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+  // CHECK-FIXES-STRICT: static float test4;
+};
+}

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

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

LGTM.

Just 2 nits, but not a blocker.

…tic data members

Improved AST Matcher based on comments
@PiotrZSL PiotrZSL merged commit 9b80ab4 into llvm:main Feb 14, 2024
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] Strict mode causes misleading redundant inline specifier warning
3 participants