diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index 36fcd8fc1b277d..1c1d3b836ea1b8 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -41,6 +41,7 @@ add_clang_library(clangTidyMiscModule UnusedParametersCheck.cpp UnusedUsingDeclsCheck.cpp UseAnonymousNamespaceCheck.cpp + UseInternalLinkageCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index d8a88324ee63e0..54bcebca7e1866 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -31,6 +31,7 @@ #include "UnusedParametersCheck.h" #include "UnusedUsingDeclsCheck.h" #include "UseAnonymousNamespaceCheck.h" +#include "UseInternalLinkageCheck.h" namespace clang::tidy { namespace misc { @@ -78,6 +79,8 @@ class MiscModule : public ClangTidyModule { "misc-unused-using-decls"); CheckFactories.registerCheck( "misc-use-anonymous-namespace"); + CheckFactories.registerCheck( + "misc-use-internal-linkage"); } }; diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp new file mode 100644 index 00000000000000..70d0281df28fad --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp @@ -0,0 +1,95 @@ +//===--- UseInternalLinkageCheck.cpp - clang-tidy--------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseInternalLinkageCheck.h" +#include "../utils/FileExtensionsUtils.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 "llvm/ADT/STLExtras.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::misc { + +namespace { + +AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); } + +static bool isInMainFile(SourceLocation L, SourceManager &SM, + const FileExtensionsSet &HeaderFileExtensions) { + for (;;) { + if (utils::isSpellingLocInHeaderFile(L, SM, HeaderFileExtensions)) + return false; + if (SM.isInMainFile(L)) + return true; + // not in header file but not in main file + L = SM.getIncludeLoc(SM.getFileID(L)); + if (L.isValid()) + continue; + // Conservative about the unknown + return false; + } +} + +AST_MATCHER_P(Decl, isAllRedeclsInMainFile, FileExtensionsSet, + HeaderFileExtensions) { + return llvm::all_of(Node.redecls(), [&](const Decl *D) { + return isInMainFile(D->getLocation(), + Finder->getASTContext().getSourceManager(), + HeaderFileExtensions); + }); +} + +AST_POLYMORPHIC_MATCHER(isExternStorageClass, + AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl)) { + return Node.getStorageClass() == SC_Extern; +} + +} // namespace + +void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) { + auto Common = + allOf(isFirstDecl(), isAllRedeclsInMainFile(HeaderFileExtensions), + unless(anyOf( + // 1. internal linkage + isStaticStorageClass(), isInAnonymousNamespace(), + // 2. explicit external linkage + isExternStorageClass(), isExternC(), + // 3. template + isExplicitTemplateSpecialization(), + // 4. friend + hasAncestor(friendDecl())))); + Finder->addMatcher( + functionDecl(Common, unless(cxxMethodDecl()), unless(isMain())) + .bind("fn"), + this); + Finder->addMatcher(varDecl(Common, hasGlobalStorage()).bind("var"), this); +} + +static constexpr StringRef Message = + "%0 %1 can be made static or moved into an anonymous namespace " + "to enforce internal linkage"; + +void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *FD = Result.Nodes.getNodeAs("fn")) { + diag(FD->getLocation(), Message) << "function" << FD; + return; + } + if (const auto *VD = Result.Nodes.getNodeAs("var")) { + diag(VD->getLocation(), Message) << "variable" << VD; + return; + } + llvm_unreachable(""); +} + +} // namespace clang::tidy::misc diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h new file mode 100644 index 00000000000000..a3c1c339659036 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h @@ -0,0 +1,38 @@ +//===--- UseInternalLinkageCheck.h - clang-tidy -----------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEINTERNALLINKAGECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEINTERNALLINKAGECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::misc { + +/// Detects variables and functions that can be marked as static or moved into +/// an anonymous namespace to enforce internal linkage. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc/use-internal-linkage.html +class UseInternalLinkageCheck : public ClangTidyCheck { +public: + UseInternalLinkageCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + HeaderFileExtensions(Context->getHeaderFileExtensions()) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + FileExtensionsSet HeaderFileExtensions; +}; + +} // namespace clang::tidy::misc + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEINTERNALLINKAGECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index da30aceb8d49d5..277a6e75da2acc 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -148,6 +148,12 @@ New checks to reading out-of-bounds data due to inadequate or incorrect string null termination. +- New :doc:`misc-use-internal-linkage + ` check. + + Detects variables and functions that can be marked as static or moved into + an anonymous namespace to enforce internal linkage. + - New :doc:`modernize-min-max-use-initializer-list ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 85e4f0352ac22b..87d3db20f76847 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -267,6 +267,7 @@ Clang-Tidy Checks :doc:`misc-unused-parameters `, "Yes" :doc:`misc-unused-using-decls `, "Yes" :doc:`misc-use-anonymous-namespace `, + :doc:`misc-use-internal-linkage `, :doc:`modernize-avoid-bind `, "Yes" :doc:`modernize-avoid-c-arrays `, :doc:`modernize-concat-nested-namespaces `, "Yes" 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 new file mode 100644 index 00000000000000..e8e43a1fb3d632 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst @@ -0,0 +1,27 @@ +.. title:: clang-tidy - misc-use-internal-linkage + +misc-use-internal-linkage +========================= + +Detects variables and functions that can be marked as static or moved into +an anonymous namespace to enforce internal linkage. + +Static functions and variables are scoped to a single file. Marking functions +and variables as static helps to better remove dead code. In addition, it gives +the compiler more information and allows for more aggressive optimizations. + +Example: + +.. code-block:: c++ + + int v1; // can be marked as static + + void fn1(); // can be marked as static + + namespace { + // already in anonymous namespace + int v2; + void fn2(); + } + // already declared as extern + extern int v2; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h new file mode 100644 index 00000000000000..0f2b576a126c4a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func.h @@ -0,0 +1,5 @@ +#pragma once + +void func_header(); + +#include "func_h.inc" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_cpp.inc b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_cpp.inc new file mode 100644 index 00000000000000..97e026f0116e99 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_cpp.inc @@ -0,0 +1 @@ +void func_cpp_inc(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc new file mode 100644 index 00000000000000..1130f710edd7c3 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/func_h.inc @@ -0,0 +1 @@ +void func_h_inc(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h new file mode 100644 index 00000000000000..37e4cfbafff146 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/var.h @@ -0,0 +1,3 @@ +#pragma once + +extern int gloabl_header; 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 new file mode 100644 index 00000000000000..c6c513fe0b0c06 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp @@ -0,0 +1,37 @@ +// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage + +#include "func.h" + +void func() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func' + +template +void func_template() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template' + +void func_cpp_inc(); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc' + +#include "func_cpp.inc" + +void func_h_inc(); + +struct S { + void method(); +}; +void S::method() {} + +void func_header(); +extern void func_extern(); +static void func_static(); +namespace { +void func_anonymous_ns(); +} // namespace + +int main(int argc, const char*argv[]) {} + +extern "C" { +void func_extern_c_1() {} +} + +extern "C" void func_extern_c_2() {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp new file mode 100644 index 00000000000000..bd5ef5431de6cc --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp @@ -0,0 +1,40 @@ +// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage + +#include "var.h" + +int global; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'global' + +template +T global_template; +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template' + +int gloabl_header; + +extern int global_extern; + +static int global_static; + +namespace { +static int global_anonymous_ns; +namespace NS { +static int global_anonymous_ns; +} +} + +static void f(int para) { + int local; + static int local_static; +} + +struct S { + int m1; + static int m2; +}; +int S::m2; + +extern "C" { +int global_in_extern_c_1; +} + +extern "C" int global_in_extern_c_2;