https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/188679
>From 32fdcb4627d00e280db2f279ff9dc337b94ddec9 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Wed, 25 Mar 2026 20:02:56 -0700 Subject: [PATCH 1/2] [clang-tidy] Make `misc-use-internal-linkage` more conservative in module interface units --- .../misc/UseInternalLinkageCheck.cpp | 21 ++++++++----- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ clang-tools-extra/test/CMakeLists.txt | 3 ++ ...internal-linkage-module-implementation.cpp | 31 +++++++++++++++++++ .../use-internal-linkage-module-partition.cpp | 10 ++++++ .../misc/use-internal-linkage-module.cpp | 21 ++++++++++++- clang-tools-extra/test/lit.cfg.py | 4 +-- 7 files changed, 84 insertions(+), 10 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-implementation.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-partition.cpp diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp index 68115cb28e7c8..3aa4b0809aa98 100644 --- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp @@ -12,6 +12,7 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/Basic/Module.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" #include "clang/Lex/Token.h" @@ -65,6 +66,14 @@ AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); } AST_MATCHER(FunctionDecl, hasBody) { return Node.hasBody(); } +AST_MATCHER(Decl, isInImportableModuleUnit) { + if (const Module *OwningModule = Node.getOwningModule()) + if (OwningModule->Kind != Module::ModuleImplementationUnit && + OwningModule->Kind != Module::PrivateModuleFragment) + return true; + return false; +} + AST_MATCHER_P(Decl, isAllRedeclsInMainFile, FileExtensionsSet, HeaderFileExtensions) { return llvm::all_of(Node.redecls(), [&](const Decl *D) { @@ -136,13 +145,9 @@ void UseInternalLinkageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { auto Common = allOf(isFirstDecl(), isAllRedeclsInMainFile(HeaderFileExtensions), - unless(anyOf( - // 1. internal linkage - isInAnonymousNamespace(), hasAncestor(decl(anyOf( - // 2. friend - friendDecl(), - // 3. module export decl - exportDecl())))))); + unless(anyOf(isInAnonymousNamespace(), isInImportableModuleUnit(), + hasAncestor(decl(friendDecl()))))); + if (AnalyzeFunctions) Finder->addMatcher( functionDecl( @@ -154,6 +159,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { isAllocationOrDeallocationOverloadedFunction(), isMain()))) .bind("fn"), this); + if (AnalyzeVariables) Finder->addMatcher( varDecl(Common, hasGlobalStorage(), @@ -163,6 +169,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { hasThreadStorageDuration()))) .bind("var"), this); + if (getLangOpts().CPlusPlus && AnalyzeTypes) Finder->addMatcher( tagDecl(Common, isDefinition(), hasNameForLinkage(), diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index daf635bf625e7..eacebb55f8c66 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -297,6 +297,10 @@ Changes in existing checks <clang-tidy/checks/misc/unused-using-decls>` to not diagnose ``using`` declarations as unused if they're exported from a module. +- Improved :doc:`misc-use-internal-linkage + <clang-tidy/checks/misc/use-internal-linkage>` to not suggest giving + internal linkage to entities defined in C++ module interface units. + - Improved :doc:`modernize-pass-by-value <clang-tidy/checks/modernize/pass-by-value>` check by adding `IgnoreMacros` option to suppress warnings in macros. diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt index 97c7a66cd69fd..22f227a891f82 100644 --- a/clang-tools-extra/test/CMakeLists.txt +++ b/clang-tools-extra/test/CMakeLists.txt @@ -44,6 +44,9 @@ set(CLANG_TOOLS_TEST_DEPS clang-resource-headers clang-tidy + + # Some tests invoke clang directly (e.g., to precompile modules). + clang ) # Add lit test dependencies. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-implementation.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-implementation.cpp new file mode 100644 index 0000000000000..9c4e00b3cf306 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-implementation.cpp @@ -0,0 +1,31 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// RUN: %clang -std=c++20 --precompile %t/foo.cppm -o %t/foo.pcm +// RUN: %check_clang_tidy -std=c++20 %t/foo.cppm misc-use-internal-linkage %t/out +// RUN: %check_clang_tidy -std=c++20 %t/foo.cpp misc-use-internal-linkage %t/out \ +// RUN: -- -- -fmodule-file=foo=%t/foo.pcm + +//--- foo.cppm + +export module foo; + +export void exported_fn(); +export extern int exported_var; +export struct ExportedStruct; + +//--- foo.cpp +module foo; + +void exported_fn() {} +int exported_var; +struct ExportedStruct {}; + +void internal_fn() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'internal_fn' can be made static or moved into an anonymous namespace to enforce internal linkage +// CHECK-FIXES: static void internal_fn() {} +int internal_var; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'internal_var' can be made static or moved into an anonymous namespace to enforce internal linkage +// CHECK-FIXES: static int internal_var; +struct InternalStruct {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'InternalStruct' can be moved into an anonymous namespace to enforce internal linkage diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-partition.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-partition.cpp new file mode 100644 index 0000000000000..8dca5bc2700c0 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module-partition.cpp @@ -0,0 +1,10 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-internal-linkage %t + +// Symbols in a partition are visible to any TU in the same module +// that imports that partition, so we shouldn't warn on them. + +module foo:bar; + +void fn_in_partition() {} +int var_in_partition; +struct StructInPartition {}; 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 ae1dc359c7ad8..9ac99bbe6e5d4 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 @@ -1,7 +1,11 @@ -// RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage +// RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-internal-linkage %t module; +void fn_in_global_module_fragment() {} +int var_in_global_module_fragment; +struct StructInGlobalModuleFragment {}; + export module test; export void single_export_fn() {} @@ -22,3 +26,18 @@ void namespace_export_fn() {} int namespace_export_var; struct NamespaceExportStruct {}; } // namespace aa + +void unexported_fn() {} +int unexported_var; +struct UnexportedStruct {}; + +module : private; + +void fn_in_private_module_fragment() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'fn_in_private_module_fragment' can be made static or moved into an anonymous namespace to enforce internal linkage +// CHECK-FIXES: static void fn_in_private_module_fragment() {} +int var_in_private_module_fragment; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'var_in_private_module_fragment' can be made static or moved into an anonymous namespace to enforce internal linkage +// CHECK-FIXES: static int var_in_private_module_fragment; +struct StructInPrivateModuleFragment {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'StructInPrivateModuleFragment' can be moved into an anonymous namespace to enforce internal linkage diff --git a/clang-tools-extra/test/lit.cfg.py b/clang-tools-extra/test/lit.cfg.py index c39ea29329674..a3901d6e3f5c9 100644 --- a/clang-tools-extra/test/lit.cfg.py +++ b/clang-tools-extra/test/lit.cfg.py @@ -49,8 +49,8 @@ # test_exec_root: The root path where tests should be run. config.test_exec_root = os.path.join(config.clang_tools_binary_dir, "test") -# Tools need the same environment setup as clang (we don't need clang itself). -llvm_config.clang_setup() +# Set up clang for use in tests. Makes %clang substitution available. +llvm_config.use_clang() if config.clang_tidy_staticanalyzer: config.available_features.add("static-analyzer") >From c84de11600468f2484723adb54c3a942dc59aa4c Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Wed, 25 Mar 2026 23:59:18 -0700 Subject: [PATCH 2/2] Address feedback --- .../clang-tidy/misc/UseInternalLinkageCheck.cpp | 5 +++-- clang-tools-extra/docs/ReleaseNotes.rst | 2 ++ .../checkers/misc/use-internal-linkage-module.cpp | 6 ------ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp index 3aa4b0809aa98..3c026c8faa37b 100644 --- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp @@ -68,8 +68,9 @@ AST_MATCHER(FunctionDecl, hasBody) { return Node.hasBody(); } AST_MATCHER(Decl, isInImportableModuleUnit) { if (const Module *OwningModule = Node.getOwningModule()) - if (OwningModule->Kind != Module::ModuleImplementationUnit && - OwningModule->Kind != Module::PrivateModuleFragment) + if (OwningModule->Kind == Module::ModuleInterfaceUnit || + OwningModule->Kind == Module::ModulePartitionInterface || + OwningModule->Kind == Module::ModulePartitionImplementation) return true; return false; } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index eacebb55f8c66..a83ef9a33aaaf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -300,6 +300,8 @@ Changes in existing checks - Improved :doc:`misc-use-internal-linkage <clang-tidy/checks/misc/use-internal-linkage>` to not suggest giving internal linkage to entities defined in C++ module interface units. + Because it only sees one file at a time, the check can't be sure + such entities aren't referenced in any other files of that module. - Improved :doc:`modernize-pass-by-value <clang-tidy/checks/modernize/pass-by-value>` check by adding `IgnoreMacros` 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 9ac99bbe6e5d4..0a64588135313 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 @@ -1,11 +1,5 @@ // RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-internal-linkage %t -module; - -void fn_in_global_module_fragment() {} -int var_in_global_module_fragment; -struct StructInGlobalModuleFragment {}; - export module test; export void single_export_fn() {} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
