Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,22 @@ UseInternalLinkageCheck::UseInternalLinkageCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
HeaderFileExtensions(Context->getHeaderFileExtensions()),
FixMode(Options.get("FixMode", FixModeKind::UseStatic)) {}
FixMode(Options.get("FixMode", FixModeKind::UseStatic)),
AnalyzeFunctions(Options.get("AnalyzeFunctions", true)),
AnalyzeVariables(Options.get("AnalyzeVariables", true)),
AnalyzeTypes(Options.get("AnalyzeTypes", true)) {
if (!AnalyzeFunctions && !AnalyzeVariables && !AnalyzeTypes)
configurationDiag(
"the 'misc-use-internal-linkage' check will not perform any "
"analysis because its 'AnalyzeFunctions', 'AnalyzeVariables', "
"and 'AnalyzeTypes' options have all been set to false");
}

void UseInternalLinkageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "FixMode", FixMode);
Options.store(Opts, "AnalyzeFunctions", AnalyzeFunctions);
Options.store(Opts, "AnalyzeVariables", AnalyzeVariables);
Options.store(Opts, "AnalyzeTypes", AnalyzeTypes);
}

void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
Expand All @@ -125,24 +137,26 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
friendDecl(),
// 3. module export decl
exportDecl()))))));
Finder->addMatcher(
functionDecl(Common, hasBody(),
unless(anyOf(isExternC(), isStaticStorageClass(),
isExternStorageClass(),
isExplicitTemplateSpecialization(),
cxxMethodDecl(), isConsteval(),
isAllocationOrDeallocationOverloadedFunction(),
isMain())))
.bind("fn"),
this);
Finder->addMatcher(varDecl(Common, hasGlobalStorage(),
unless(anyOf(isExternC(), isStaticStorageClass(),
isExternStorageClass(),
isExplicitTemplateSpecialization(),
hasThreadStorageDuration())))
.bind("var"),
this);
if (getLangOpts().CPlusPlus)
if (AnalyzeFunctions)
Finder->addMatcher(
functionDecl(
Common, hasBody(),
unless(anyOf(
isExternC(), isStaticStorageClass(), isExternStorageClass(),
isExplicitTemplateSpecialization(), cxxMethodDecl(),
isConsteval(), isAllocationOrDeallocationOverloadedFunction(),
isMain())))
.bind("fn"),
this);
if (AnalyzeVariables)
Finder->addMatcher(varDecl(Common, hasGlobalStorage(),
unless(anyOf(isExternC(), isStaticStorageClass(),
isExternStorageClass(),
isExplicitTemplateSpecialization(),
hasThreadStorageDuration())))
.bind("var"),
this);
if (getLangOpts().CPlusPlus && AnalyzeTypes)
Finder->addMatcher(
tagDecl(Common, isDefinition(), hasNameForLinkage(),
hasDeclContext(anyOf(translationUnitDecl(), namespaceDecl())),
Expand Down
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ class UseInternalLinkageCheck : public ClangTidyCheck {
private:
FileExtensionsSet HeaderFileExtensions;
FixModeKind FixMode;
const bool AnalyzeFunctions;
const bool AnalyzeVariables;
const bool AnalyzeTypes;
};

} // namespace clang::tidy::misc
Expand Down
4 changes: 3 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,9 @@ Changes in existing checks

- Improved :doc:`misc-use-internal-linkage
<clang-tidy/checks/misc/use-internal-linkage>` to suggest giving
structs, classes, unions, and enums internal linkage.
user-defined types (structs, classes, unions, and enums) internal
linkage. Added fine-grained options to control whether the check
should diagnose functions, variables, and/or user-defined types.

- Improved :doc:`modernize-avoid-c-arrays
<clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Example:
int v1; // can be marked as static

void fn1() {} // can be marked as static

struct S1 {}; // can be moved into anonymous namespace

namespace {
Expand Down Expand Up @@ -50,3 +50,16 @@ Options

- `UseStatic`
Add ``static`` for internal linkage variable and function.

.. option:: AnalyzeFunctions

Whether to suggest giving functions internal linkage. Default is `true`.

.. option:: AnalyzeVariables

Whether to suggest giving variables internal linkage. Default is `true`.

.. option:: AnalyzeTypes

(C++-only) Whether to suggest giving user-defined types (structs,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'm not sure if (C++-only) is the preferred wording here. I grepped the codebase and found that (C++ only) (without the hyphen) is much more common in comments (e.g. clang/lib/Sema/SemaType.cpp, clang/include/clang/AST/Decl.h).

Maybe we could simply drop the hyphen or switch to natural language similar to modernize-unary-static-assert

classes, unions, and enums) internal linkage. Default is `true`.
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
// RUN: }}" -- -I%S/Inputs/use-internal-linkage

// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
// RUN: }}" -- -I%S/Inputs/use-internal-linkage

#include "func.h"

Expand Down Expand Up @@ -57,7 +66,6 @@ NNDS void func_nnds() {}
void func_h_inc() {}

struct S {
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage
void method();
};
void S::method() {}
Expand Down Expand Up @@ -96,3 +104,6 @@ void * operator new[](std::size_t) { return nullptr; }
void operator delete(void*) noexcept {}
void operator delete[](void*) noexcept {}
// gh117489 end

int ignored_global = 0; // AnalyzeVariables is false.
struct IgnoredStruct {}; // AnalyzeTypes is false.
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally named this file ...-tag.cpp because it was testing TagDecls, but I've renamed it to ...-type.cpp to match the option AnalyzeTypes.

// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.AnalyzeFunctions: false \
// RUN: }}" -- -I%S/Inputs/use-internal-linkage

#include "tag.h"
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
// RUN: misc-use-internal-linkage.AnalyzeFunctions: false \
// RUN: }}" -- -I%S/Inputs/use-internal-linkage

#include "type.h"

struct StructDeclaredInHeader {};
union UnionDeclaredInHeader {};
Expand Down Expand Up @@ -56,7 +63,6 @@ struct OuterStruct {
struct OuterStruct::InnerStructDefinedOutOfLine {};

void f() {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' can be made static or moved into an anonymous namespace to enforce internal linkage
struct StructInsideFunction {};
}

Expand Down Expand Up @@ -97,3 +103,5 @@ extern "C" {
struct InExternCBlock { int i; };

}

void ignored_func() {} // AnalyzeFunctions is false.
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
// RUN: }}" -- -I%S/Inputs/use-internal-linkage

// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
// RUN: }}" -- -I%S/Inputs/use-internal-linkage

#include "var.h"

Expand Down Expand Up @@ -47,7 +56,6 @@ static void f(int para) {
}

struct S {
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage
int m1;
static int m2;
};
Expand All @@ -61,3 +69,6 @@ extern "C" int global_in_extern_c_2;

const int const_global = 123;
constexpr int constexpr_global = 123;

struct IgnoredStruct {}; // AnalyzeTypes is false.
void ignored_func() {} // AnalyzeFunctions is false.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
// RUN: -config="{CheckOptions: { \
// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
// RUN: }}"

// CHECK-MESSAGES: warning: the 'misc-use-internal-linkage' check will not perform any analysis because its 'AnalyzeFunctions', 'AnalyzeVariables', and 'AnalyzeTypes' options have all been set to false [clang-tidy-config]