https://github.com/localspook created https://github.com/llvm/llvm-project/pull/172797
Currently, the check only supports variables and functions. This PR extends it to classes (and structs, unions, enums, etc.). >From 4322a1154194444fcf5fb94395b70acd15ac117f Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Wed, 17 Dec 2025 21:37:16 -0800 Subject: [PATCH] [clang-tidy] Teach `misc-use-internal-linkage` to suggest giving classes internal linkage --- .../misc/UseInternalLinkageCheck.cpp | 48 ++++++---- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../checks/misc/use-internal-linkage.rst | 14 +-- .../misc/Inputs/use-internal-linkage/tag.h | 9 ++ .../misc/use-internal-linkage-func.cpp | 1 + .../misc/use-internal-linkage-module.cpp | 4 + .../misc/use-internal-linkage-tag.cpp | 90 +++++++++++++++++++ .../misc/use-internal-linkage-var.cpp | 1 + 8 files changed, 151 insertions(+), 20 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp index bad51c600f1cb..143525fdaa404 100644 --- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp @@ -96,6 +96,8 @@ AST_MATCHER(FunctionDecl, isAllocationOrDeallocationOverloadedFunction) { return OverloadedOperators.contains(Node.getOverloadedOperator()); } +AST_MATCHER(TagDecl, hasNameForLinkage) { return Node.hasNameForLinkage(); } + } // namespace UseInternalLinkageCheck::UseInternalLinkageCheck(StringRef Name, @@ -113,27 +115,37 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { allOf(isFirstDecl(), isAllRedeclsInMainFile(HeaderFileExtensions), unless(anyOf( // 1. internal linkage - isStaticStorageClass(), isInAnonymousNamespace(), - // 2. explicit external linkage - isExternStorageClass(), isExternC(), - // 3. template - isExplicitTemplateSpecialization(), - hasAncestor(decl(anyOf( - // 4. friend - friendDecl(), - // 5. module export decl - exportDecl())))))); + isInAnonymousNamespace(), hasAncestor(decl(anyOf( + // 2. friend + friendDecl(), + // 3. module export decl + exportDecl())))))); Finder->addMatcher( functionDecl(Common, hasBody(), - unless(anyOf(cxxMethodDecl(), isConsteval(), + unless(anyOf(isExternC(), isStaticStorageClass(), + isExternStorageClass(), + isExplicitTemplateSpecialization(), + cxxMethodDecl(), isConsteval(), isAllocationOrDeallocationOverloadedFunction(), isMain()))) .bind("fn"), this); - Finder->addMatcher( - varDecl(Common, hasGlobalStorage(), unless(hasThreadStorageDuration())) - .bind("var"), - this); + Finder->addMatcher(varDecl(Common, hasGlobalStorage(), + unless(anyOf(isExternC(), isStaticStorageClass(), + isExternStorageClass(), + isExplicitTemplateSpecialization(), + hasThreadStorageDuration()))) + .bind("var"), + this); + if (getLangOpts().CPlusPlus) + Finder->addMatcher( + tagDecl( + Common, isDefinition(), hasNameForLinkage(), + hasDeclContext(anyOf(translationUnitDecl(), namespaceDecl())), + unless(anyOf(classTemplatePartialSpecializationDecl(), + cxxRecordDecl(isExplicitTemplateSpecialization())))) + .bind("tag"), + this); } static constexpr StringRef Message = @@ -167,6 +179,12 @@ void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) { DB << FixItHint::CreateInsertion(FixLoc, "static "); return; } + if (const auto *TD = Result.Nodes.getNodeAs<TagDecl>("tag")) { + diag(TD->getLocation(), "%0 %1 can be moved into an anonymous namespace " + "to enforce internal linkage") + << TD->getKindName() << TD; + return; + } llvm_unreachable(""); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 924b2c03cfd18..c4328e9e95ca8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -517,6 +517,10 @@ Changes in existing checks - Improved :doc:`misc-header-include-cycle <clang-tidy/checks/misc/header-include-cycle>` check performance. +- Improved :doc:`misc-use-internal-linkage + <clang-tidy/checks/misc/use-internal-linkage>` to suggest giving + structs, classes, unions, and enums internal linkage. + - Improved :doc:`modernize-avoid-c-arrays <clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types which are part of an implicit instantiation of a template. diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst index 224ad21ecc5c3..8838837506a40 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst @@ -3,12 +3,12 @@ misc-use-internal-linkage ========================= -Detects variables and functions that can be marked as static or moved into -an anonymous namespace to enforce internal linkage. +Detects variables, functions, and classes that can be marked as static or +moved into an anonymous namespace to enforce internal linkage. -Static functions and variables are scoped to a single file. Marking functions -and variables as static helps to better remove dead code. In addition, it gives -the compiler more information and allows for more aggressive optimizations. +Any entity that's only used within a single file should be given internal +linkage. Doing so gives the compiler more information, allowing it to better +remove dead code and perform more aggressive optimizations. Example: @@ -17,11 +17,14 @@ Example: int v1; // can be marked as static void fn1() {} // can be marked as static + + struct S1 {}; // can be moved into anonymous namespace namespace { // already in anonymous namespace int v2; void fn2(); + struct S2 {}; } // already declared as extern extern int v2; @@ -33,6 +36,7 @@ Example: export void fn4() {} export namespace t { void fn5() {} } export int v2; + export class C {}; Options ------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h new file mode 100644 index 0000000000000..b6bddaf8e8d0e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h @@ -0,0 +1,9 @@ +#pragma once + +enum EnumDeclaredInHeader : int; +struct StructDeclaredInHeader; +union UnionDeclaredInHeader; +class ClassDeclaredInHeader; + +template <typename> +class TemplateDeclaredInHeader {}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp index abf95b857c192..1709ebc5f7144 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp @@ -57,6 +57,7 @@ NNDS void func_nnds() {} void func_h_inc() {} struct S { +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage void method(); }; void S::method() {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp index 9b0d8cde429b6..aff978f1619d9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp @@ -6,15 +6,19 @@ export module test; export void single_export_fn() {} export int single_export_var; +export struct SingleExportStruct {}; export { void group_export_fn1() {} void group_export_fn2() {} int group_export_var1; int group_export_var2; + struct GroupExportStruct1 {}; + struct GroupExportStruct2 {}; } export namespace aa { void namespace_export_fn() {} int namespace_export_var; +struct NamespaceExportStruct {}; } // namespace aa diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp new file mode 100644 index 0000000000000..29379f4c40dc1 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp @@ -0,0 +1,90 @@ +// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage +// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \ +// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage + +#include "tag.h" + +struct StructDeclaredInHeader {}; +union UnionDeclaredInHeader {}; +class ClassDeclaredInHeader {}; +enum EnumDeclaredInHeader : int {}; +template <typename T> class TemplateDeclaredInHeader<T *> {}; +template <> class TemplateDeclaredInHeader<int> {}; + +struct StructWithNoDefinition; +union UnionWithNoDefinition; +class ClassWithNoDefinition; +enum EnumWithNoDefinition : int; + +namespace { + +struct StructAlreadyInAnonymousNamespace {}; +union UnionAlreadyInAnonymousNamespace {}; +class ClassAlreadyInAnonymousNamespace {}; +enum EnumAlreadyInAnonymousNamespace : int {}; +typedef struct {} TypedefedStructAlreadyInAnonymousNamespace; + +} // namespace + +struct S {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage +union U {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: union 'U' can be moved into an anonymous namespace to enforce internal linkage +class C {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C' can be moved into an anonymous namespace to enforce internal linkage +enum E {}; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'E' can be moved into an anonymous namespace to enforce internal linkage + +template <typename> +class Template {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'Template' can be moved into an anonymous namespace to enforce internal linkage + +struct OuterStruct { +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'OuterStruct' can be moved into an anonymous namespace to enforce internal linkage + + // No warnings for the inner members. + struct InnerStruct {}; + union InnerUnion {}; + class InnerClass {}; + enum InnerEnum {}; + struct InnerStructDefinedOutOfLine; +}; +struct OuterStruct::InnerStructDefinedOutOfLine {}; + +void f() { +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' can be made static or moved into an anonymous namespace to enforce internal linkage + struct StructInsideFunction {}; +} + +namespace ns { + +struct S {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage +union U {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: union 'U' can be moved into an anonymous namespace to enforce internal linkage +class C {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class 'C' can be moved into an anonymous namespace to enforce internal linkage +enum E {}; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'E' can be moved into an anonymous namespace to enforce internal linkage + +} // namespace ns + +typedef struct {} TypedefedStruct; +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: struct 'TypedefedStruct' can be moved into an anonymous namespace to enforce internal linkage + +struct Named {} Variable; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'Named' can be moved into an anonymous namespace to enforce internal linkage +// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: variable 'Variable' can be made static or moved into an anonymous namespace to enforce internal linkage +// CHECK-FIXES: static struct Named {} Variable; + +struct {} VariableOfUnnamedType; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: variable 'VariableOfUnnamedType' can be made static or moved into an anonymous namespace to enforce internal linkage +// CHECK-FIXES: static struct {} VariableOfUnnamedType; + +extern "C" struct MarkedExternC { int i; }; + +extern "C" { + +struct InExternCBlock { int i; }; + +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp index 3da05c71dd94f..e30fc7b54953e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp @@ -47,6 +47,7 @@ static void f(int para) { } struct S { +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage int m1; static int m2; }; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
