[clang-tools-extra] [clang-tidy] fix false positive use-internal-linkage for func decl without body (PR #117490)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

<details>
<summary>Changes</summary>

If in one TU, function only have declaration without body, this function should be external linkage.
Fixed #<!-- -->117488


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


4 Files Affected:

- (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp (+3-3) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-2) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst (+4-1) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp (+33-23) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index d900978f65a944..71eb2d94cd4f26 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -8,14 +8,12 @@
 
 #include "UseInternalLinkageCheck.h"
 #include "../utils/FileExtensionsUtils.h"
-#include "../utils/LexerUtils.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchersMacros.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
-#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/STLExtras.h"
 
@@ -47,6 +45,8 @@ namespace {
 
 AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); }
 
+AST_MATCHER(FunctionDecl, hasBody) { return Node.hasBody(); }
+
 static bool isInMainFile(SourceLocation L, SourceManager &SM,
                          const FileExtensionsSet &HeaderFileExtensions) {
   for (;;) {
@@ -103,7 +103,7 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
                 // 4. friend
                 hasAncestor(friendDecl()))));
   Finder->addMatcher(
-      functionDecl(Common, unless(cxxMethodDecl()), unless(isMain()))
+      functionDecl(Common, hasBody(), unless(cxxMethodDecl()), unless(isMain()))
           .bind("fn"),
       this);
   Finder->addMatcher(varDecl(Common, hasGlobalStorage()).bind("var"), this);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8c9fedf4b4406c..1f338f23f4e6ea 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -229,8 +229,9 @@ Changes in existing checks
   false positive for C++23 deducing this.
 
 - Improved :doc:`misc-use-internal-linkage
-  <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static`` keyword
-  before type qualifiers such as ``const`` and ``volatile``.
+  <clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static``
+  keyword before type qualifiers such as ``const`` and ``volatile`` and fix
+  false positives for function declaration without body.
 
 - Improved :doc:`modernize-avoid-c-arrays
   <clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using 
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 7147af9a7919bc..b8bbcc62706101 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
@@ -16,7 +16,7 @@ Example:
 
   int v1; // can be marked as static
 
-  void fn1(); // can be marked as static
+  void fn1() {} // can be marked as static
 
   namespace {
     // already in anonymous namespace
@@ -26,6 +26,9 @@ Example:
   // already declared as extern
   extern int v2;
 
+  void fn3(); // without function body in all declaration, maybe external linkage
+  void fn3();
+
 Options
 -------
 
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 8dc739da3a2734..bf0d2c2513e562 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
@@ -13,59 +13,59 @@ void func_template() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template'
 // CHECK-FIXES: static void func_template() {}
 
-void func_cpp_inc();
+void func_cpp_inc() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc'
-// CHECK-FIXES: static void func_cpp_inc();
+// CHECK-FIXES: static void func_cpp_inc() {}
 
-int* func_cpp_inc_return_ptr();
+int* func_cpp_inc_return_ptr() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc_return_ptr'
-// CHECK-FIXES: static int* func_cpp_inc_return_ptr();
+// CHECK-FIXES: static int* func_cpp_inc_return_ptr() {}
 
-const int* func_cpp_inc_return_const_ptr();
+const int* func_cpp_inc_return_const_ptr() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_const_ptr'
-// CHECK-FIXES: static const int* func_cpp_inc_return_const_ptr();
+// CHECK-FIXES: static const int* func_cpp_inc_return_const_ptr() {}
 
-int const* func_cpp_inc_return_ptr_const();
+int const* func_cpp_inc_return_ptr_const() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_ptr_const'
-// CHECK-FIXES: static int const* func_cpp_inc_return_ptr_const();
+// CHECK-FIXES: static int const* func_cpp_inc_return_ptr_const() {}
 
-int * const func_cpp_inc_return_const();
+int * const func_cpp_inc_return_const() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'func_cpp_inc_return_const'
-// CHECK-FIXES: static int * const func_cpp_inc_return_const();
+// CHECK-FIXES: static int * const func_cpp_inc_return_const() {}
 
-volatile const int* func_cpp_inc_return_volatile_const_ptr();
+volatile const int* func_cpp_inc_return_volatile_const_ptr() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'func_cpp_inc_return_volatile_const_ptr'
-// CHECK-FIXES: static volatile const int* func_cpp_inc_return_volatile_const_ptr();
+// CHECK-FIXES: static volatile const int* func_cpp_inc_return_volatile_const_ptr() {}
 
-[[nodiscard]] void func_nodiscard();
+[[nodiscard]] void func_nodiscard() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function 'func_nodiscard'
-// CHECK-FIXES: {{\[\[nodiscard\]\]}} static void func_nodiscard();
+// CHECK-FIXES: {{\[\[nodiscard\]\]}} static void func_nodiscard() {}
 
 #define NDS [[nodiscard]]
 #define NNDS
 
-NDS void func_nds();
+NDS void func_nds() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function 'func_nds'
-// CHECK-FIXES: NDS static void func_nds();
+// CHECK-FIXES: NDS static void func_nds() {}
 
-NNDS void func_nnds();
+NNDS void func_nnds() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'func_nnds'
-// CHECK-FIXES: NNDS static void func_nnds();
+// CHECK-FIXES: NNDS static void func_nnds() {}
 
 #include "func_cpp.inc"
 
-void func_h_inc();
+void func_h_inc() {}
 
 struct S {
   void method();
 };
 void S::method() {}
 
-void func_header();
-extern void func_extern();
-static void func_static();
+void func_header() {}
+extern void func_extern() {}
+static void func_static() {}
 namespace {
-void func_anonymous_ns();
+void func_anonymous_ns() {}
 } // namespace
 
 int main(int argc, const char*argv[]) {}
@@ -75,3 +75,13 @@ void func_extern_c_1() {}
 }
 
 extern "C" void func_extern_c_2() {}
+
+namespace gh117488 {
+void func_with_body();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_with_body'
+// CHECK-FIXES: static void func_with_body();
+void func_with_body() {}
+
+void func_without_body();
+void func_without_body();
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/117490