llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) <details> <summary>Changes</summary> There were several requests in #<!-- -->172797 to add fine-grained configuration for this check: here it is. By default, everything is enabled. --- Full diff: https://github.com/llvm/llvm-project/pull/173310.diff 9 Files Affected: - (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp (+33-19) - (modified) clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h (+3) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst (+14-1) - (renamed) clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h () - (modified) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp (+14-3) - (renamed) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp (+12-4) - (modified) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp (+14-3) - (added) clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp (+8) ``````````diff diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp index 0bfb5cd52353a..617b98484f3e4 100644 --- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp @@ -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) { @@ -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())), diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h index eedb5bd1adcab..5ab3eae515a0f 100644 --- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h +++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h @@ -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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e4ff640811933..5bf2177ed5941 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -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 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 8838837506a40..da094fd5ec0e0 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 @@ -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 { @@ -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, + classes, unions, and enums) internal linkage. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h similarity index 100% rename from clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h rename to clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h 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 1709ebc5f7144..d25e4383613f7 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 @@ -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" @@ -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() {} @@ -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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp similarity index 88% rename from clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp rename to clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp index 55bdfd4ca36c6..771d7e9d422c4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp @@ -1,8 +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: }}" -- -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 {}; @@ -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 {}; } @@ -97,3 +103,5 @@ extern "C" { struct InExternCBlock { int i; }; } + +void ignored_func() {} // AnalyzeFunctions is false. 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 index e30fc7b54953e..1be7165f9ffe6 100644 --- 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 @@ -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" @@ -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; }; @@ -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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp new file mode 100644 index 0000000000000..a99f17e5ddc0a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp @@ -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] `````````` </details> https://github.com/llvm/llvm-project/pull/173310 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
