Author: Victor Chernyakin Date: 2025-12-27T09:51:21-07:00 New Revision: c381a0918a2e929e573c35e8e88b799a4b6e3195
URL: https://github.com/llvm/llvm-project/commit/c381a0918a2e929e573c35e8e88b799a4b6e3195 DIFF: https://github.com/llvm/llvm-project/commit/c381a0918a2e929e573c35e8e88b799a4b6e3195.diff LOG: [clang-tidy] Add C support to `misc-use-internal-linkage` (#173196) Right now, this check simply doesn't work in C, because we exclude anything that `isExternC` from analysis (in C, everything `isExternC`). Besides that, the docs and diagnostic message talk about anonymous namespaces, which don't exist in C (this was noted in #97969, I'm just summarizing). The existing tests use abbreviated `// CHECK-MESSAGES` assertions (e.g. `// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: function 'cxf'`), but I've expanded them out. Yes, it's verbose, but now that the diagnostic message has an important difference between C and C++, I feel it's important that we test it. Added: clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage.c Modified: clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-consteval.cpp clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp index 617b98484f3e4..68115cb28e7c8 100644 --- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp @@ -96,6 +96,12 @@ AST_MATCHER(FunctionDecl, isAllocationOrDeallocationOverloadedFunction) { return OverloadedOperators.contains(Node.getOverloadedOperator()); } +AST_POLYMORPHIC_MATCHER(isExplicitlyExternC, + AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl)) { + return Finder->getASTContext().getLangOpts().CPlusPlus && Node.isExternC(); +} + AST_MATCHER(TagDecl, hasNameForLinkage) { return Node.hasNameForLinkage(); } AST_MATCHER(CXXRecordDecl, isExplicitTemplateInstantiation) { @@ -142,20 +148,21 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { functionDecl( Common, hasBody(), unless(anyOf( - isExternC(), isStaticStorageClass(), isExternStorageClass(), - isExplicitTemplateSpecialization(), cxxMethodDecl(), - isConsteval(), isAllocationOrDeallocationOverloadedFunction(), - isMain()))) + isExplicitlyExternC(), isStaticStorageClass(), + isExternStorageClass(), isExplicitTemplateSpecialization(), + cxxMethodDecl(), isConsteval(), + isAllocationOrDeallocationOverloadedFunction(), isMain()))) .bind("fn"), this); if (AnalyzeVariables) - Finder->addMatcher(varDecl(Common, hasGlobalStorage(), - unless(anyOf(isExternC(), isStaticStorageClass(), - isExternStorageClass(), - isExplicitTemplateSpecialization(), - hasThreadStorageDuration()))) - .bind("var"), - this); + Finder->addMatcher( + varDecl(Common, hasGlobalStorage(), + unless(anyOf(isExplicitlyExternC(), isStaticStorageClass(), + isExternStorageClass(), + isExplicitTemplateSpecialization(), + hasThreadStorageDuration()))) + .bind("var"), + this); if (getLangOpts().CPlusPlus && AnalyzeTypes) Finder->addMatcher( tagDecl(Common, isDefinition(), hasNameForLinkage(), @@ -169,13 +176,13 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { } static constexpr StringRef Message = - "%0 %1 can be made static or moved into an anonymous namespace " + "%0 %1 can be made static %select{|or moved into an anonymous namespace }2" "to enforce internal linkage"; void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn")) { const DiagnosticBuilder DB = diag(FD->getLocation(), Message) - << "function" << FD; + << "function" << FD << getLangOpts().CPlusPlus; const SourceLocation FixLoc = FD->getInnerLocStart(); if (FixLoc.isInvalid() || FixLoc.isMacroID()) return; @@ -191,7 +198,7 @@ void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) { return; const DiagnosticBuilder DB = diag(VD->getLocation(), Message) - << "variable" << VD; + << "variable" << VD << getLangOpts().CPlusPlus; const SourceLocation FixLoc = VD->getInnerLocStart(); if (FixLoc.isInvalid() || FixLoc.isMacroID()) return; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7b1640594a3d3..86bfd1d489898 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -528,6 +528,7 @@ Changes in existing checks user-defined types (structs, classes, unions, and enums) internal linkage. Added fine-grained options to control whether the check should diagnose functions, variables, and/or user-defined types. + Enabled the check for C. - Improved :doc:`modernize-avoid-c-arrays <clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types 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 941221573fc86..9c160756e3873 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 @@ -4,7 +4,7 @@ misc-use-internal-linkage ========================= Detects variables, functions, and classes that can be marked as static or -moved into an anonymous namespace to enforce internal linkage. +(in C++) moved into an anonymous namespace to enforce internal linkage. 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 @@ -18,6 +18,14 @@ Example: void fn1() {} // can be marked as static + // already declared as extern + extern int v2; + + void fn3(); // without function body in all declaration, maybe external linkage + void fn3(); + + // === C++-specific === + struct S1 {}; // can be moved into anonymous namespace namespace { @@ -26,11 +34,6 @@ Example: void fn2(); struct S2 {}; } - // already declared as extern - extern int v2; - - void fn3(); // without function body in all declaration, maybe external linkage - void fn3(); // export declarations export void fn4() {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h index 0f2b576a126c4..c0f967523d1c3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h @@ -1,5 +1,5 @@ #pragma once -void func_header(); +void func_header(void); #include "func_h.inc" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc index 1130f710edd7c..bd29019080de3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc @@ -1 +1 @@ -void func_h_inc(); +void func_h_inc(void); diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h index 37e4cfbafff14..844c61a1135af 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h @@ -1,3 +1,3 @@ #pragma once -extern int gloabl_header; +extern int global_header; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-consteval.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-consteval.cpp index 62c9818e07c4f..b63e87da25cea 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-consteval.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-consteval.cpp @@ -3,5 +3,5 @@ consteval void gh122096() {} constexpr void cxf() {} -// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: function 'cxf' +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: function 'cxf' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static constexpr void cxf() {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp index 3f2f5897bf718..8eb4a40d2d7d6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-fix-mode-none.cpp @@ -2,9 +2,9 @@ // RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'None'}}" -- -I%S/Inputs/use-internal-linkage void func() {} -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES-NOT: static void func() {} int global; -// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global' +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES-NOT: static int global; 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 d25e4383613f7..764208443bdc1 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 @@ -14,51 +14,51 @@ #include "func.h" void func() {} -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static void func() {} template<class T> void func_template() {} -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static void func_template() {} void func_cpp_inc() {} -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static void func_cpp_inc() {} int* func_cpp_inc_return_ptr() { return nullptr; } -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc_return_ptr' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc_return_ptr' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static int* func_cpp_inc_return_ptr() { return nullptr; } const int* func_cpp_inc_return_const_ptr() { return nullptr; } -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_const_ptr' +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_const_ptr' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static const int* func_cpp_inc_return_const_ptr() { return nullptr; } int const* func_cpp_inc_return_ptr_const() { return nullptr; } -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_ptr_const' +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_ptr_const' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static int const* func_cpp_inc_return_ptr_const() { return nullptr; } int * const func_cpp_inc_return_const() { return nullptr; } -// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'func_cpp_inc_return_const' +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'func_cpp_inc_return_const' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static int * const func_cpp_inc_return_const() { return nullptr; } volatile const int* func_cpp_inc_return_volatile_const_ptr() { return nullptr; } -// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'func_cpp_inc_return_volatile_const_ptr' +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'func_cpp_inc_return_volatile_const_ptr' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static volatile const int* func_cpp_inc_return_volatile_const_ptr() { return nullptr; } [[nodiscard]] void func_nodiscard() {} -// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function 'func_nodiscard' +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function 'func_nodiscard' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: {{\[\[nodiscard\]\]}} static void func_nodiscard() {} #define NDS [[nodiscard]] #define NNDS NDS void func_nds() {} -// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function 'func_nds' +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function 'func_nds' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: NDS static void func_nds() {} NNDS void func_nnds() {} -// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'func_nnds' +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'func_nnds' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: NNDS static void func_nnds() {} #include "func_cpp.inc" @@ -87,7 +87,7 @@ extern "C" void func_extern_c_2() {} namespace gh117488 { void func_with_body(); -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_with_body' +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_with_body' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static void func_with_body(); void func_with_body() {} 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 1be7165f9ffe6..7de5259a0a160 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 @@ -14,27 +14,27 @@ #include "var.h" int global; -// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global' +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static int global; template<class T> T global_template; -// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template' +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static T global_template; int const* ptr_const_star; -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'ptr_const_star' +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'ptr_const_star' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static int const* ptr_const_star; const int* const_ptr_star; -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'const_ptr_star' +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'const_ptr_star' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static const int* const_ptr_star; const volatile int* const_volatile_ptr_star; -// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: variable 'const_volatile_ptr_star' +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: variable 'const_volatile_ptr_star' can be made static or moved into an anonymous namespace to enforce internal linkage // CHECK-FIXES: static const volatile int* const_volatile_ptr_star; -int gloabl_header; +int global_header; extern int global_extern; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage.c b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage.c new file mode 100644 index 0000000000000..05f9349ea3214 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage.c @@ -0,0 +1,33 @@ +// 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 "func.h" + +void func(void) {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func' can be made static to enforce internal linkage +// CHECK-FIXES: static void func(void) {} + +void func_header(void) {} +extern void func_extern(void) {} +static void func_static(void) {} + +int main(void) {} + + +#include "var.h" + +int global; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global' can be made static to enforce internal linkage +// CHECK-FIXES: static int global; + +const int const_global = 123; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: variable 'const_global' can be made static to enforce internal linkage +// CHECK-FIXES: static const int const_global = 123; + +int global_header; +extern int global_extern; +static int global_static; +#if __STDC_VERSION__ >= 201112L +_Thread_local int global_thread_local; +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
