[clang-tidy] Make misc-use-internal-linkage not diagnose symbols in importable module units (llvm#188679)

Fixes llvm#187884.

Author: localspook

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,15 @@ 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::ModuleInterfaceUnit ||
+        OwningModule->Kind == Module::ModulePartitionInterface ||
+        OwningModule->Kind == Module::ModulePartitionImplementation)
+      return true;
+  return false;
+}
+
 AST_MATCHER_P(Decl, isAllRedeclsInMainFile, FileExtensionsSet,
               HeaderFileExtensions) {
   return llvm::all_of(Node.redecls(), [&](const Decl *D) {
@@ -136,13 +146,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 +160,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                 isAllocationOrDeallocationOverloadedFunction(), isMain())))
             .bind("fn"),
         this);
+
   if (AnalyzeVariables)
     Finder->addMatcher(
         varDecl(Common, hasGlobalStorage(),
@@ -163,6 +170,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                              hasThreadStorageDuration())))
             .bind("var"),
         this);
+
   if (getLangOpts().CPlusPlus && AnalyzeTypes)
     Finder->addMatcher(
         tagDecl(Common, isDefinition(), hasNameForLinkage(),

clang-tools-extra/docs/ReleaseNotes.rst

@@ -350,6 +350,12 @@ 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.
+  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`
   option to suppress warnings in macros.

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

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 {};

clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-module.cpp

@@ -1,6 +1,4 @@
-// RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
-
-module;
+// RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-internal-linkage %t
 
 export module test;
 
@@ -22,3 +20,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