Author: Congcong Cai Date: 2024-11-25T06:46:10+08:00 New Revision: e3aafe407af36f580148d3ceccd1aa13a490f7c9
URL: https://github.com/llvm/llvm-project/commit/e3aafe407af36f580148d3ceccd1aa13a490f7c9 DIFF: https://github.com/llvm/llvm-project/commit/e3aafe407af36f580148d3ceccd1aa13a490f7c9.diff LOG: [clang-tidy] fix false positive use-internal-linkage for func decl without body (#117490) If in one TU, function only have declaration without body, this function should be external linkage. Fixed #117488 Added: 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/use-internal-linkage-func.cpp Removed: ################################################################################ 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(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
