Skip to content
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

[clang-tidy] Fix fix-it overlaps in readability-static-definition-in-anonymous-namespace #86599

Conversation

PiotrZSL
Copy link
Member

Because check emitted multiple warnings for every template instance fix-it couldn't be applied due to overlaps.

Using TK_IgnoreUnlessSpelledInSource and restricting check to C++ only.

…anonymous-namespace

Because check emited multiple warnings for every template instance
fix-it couldn't be applied due to overlaps.

Using TK_IgnoreUnlessSpelledInSource and restricting check to C++ only.
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2024

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

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Because check emitted multiple warnings for every template instance fix-it couldn't be applied due to overlaps.

Using TK_IgnoreUnlessSpelledInSource and restricting check to C++ only.


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.h (+6)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/static-definition-in-anonymous-namespace.cpp (+11)
diff --git a/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.h b/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.h
index c28087e07e9b61..620cd6e3f2f877 100644
--- a/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.h
@@ -24,6 +24,12 @@ class StaticDefinitionInAnonymousNamespaceCheck : public ClangTidyCheck {
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..a60a525bc3aec2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -249,6 +249,11 @@ Changes in existing checks
   mode by resolving symbolic links to header files. Fixed handling of Hungarian
   Prefix when configured to `LowerCase`.
 
+- Improved :doc:`readability-static-definition-in-anonymous-namespace
+  <clang-tidy/checks/readability/static-definition-in-anonymous-namespace>`
+  check by resolving fix-it overlaps in template code by disregarding implicit
+  instances.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/static-definition-in-anonymous-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/static-definition-in-anonymous-namespace.cpp
index e9938db4f5b83f..e204199393db42 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/static-definition-in-anonymous-namespace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/static-definition-in-anonymous-namespace.cpp
@@ -51,6 +51,17 @@ static int c = 1;
 } // namespace deep_inner
 } // namespace inner
 
+template<typename T>
+static void printTemplate(T&&) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 'printTemplate' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}void printTemplate(T&&) {}
+
+void testTemplate() {
+  printTemplate(5);
+  printTemplate(5U);
+  printTemplate("some string");
+}
+
 } // namespace
 
 namespace N {

Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

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

LGTM

@PiotrZSL PiotrZSL merged commit 3137347 into llvm:main Mar 26, 2024
8 checks passed
@PiotrZSL PiotrZSL deleted the fix-overlaps-in-readability-static-definition-in-anonymous-namespace branch March 26, 2024 17:20
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.

None yet

3 participants