[clang-tidy] fix false positive use-internal-linkage for func decl without body #117490
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…thout body If in one TU, function only have declaration without body, this function should be external linkage. Fixed llvm#117488
Member
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Congcong Cai (HerrCai0907) ChangesIf in one TU, function only have declaration without body, this function should be external linkage. Full diff: https://github.com/llvm/llvm-project/pull/117490.diff 4 Files Affected:
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();
+}
|
5chmidti
approved these changes
Nov 24, 2024
HerrCai0907
deleted the
117488-clang-tidy-misc-use-internal-linkage-false-positive-for-function-declaration-without-body
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If in one TU, function only have declaration without body, this function should be external linkage.
Fixed #117488