https://github.com/Discookie updated https://github.com/llvm/llvm-project/pull/117165
>From e90ab99dde0945d103959fa73ea2d31852f753e7 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Thu, 21 Nov 2024 14:33:24 +0000 Subject: [PATCH 1/2] [clang-tidy] Add C++ member function support to user-defined bugprone-unsafe-functions matches --- .../bugprone/UnsafeFunctionsCheck.cpp | 38 ++++++++++++++----- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../unsafe-functions-custom-regex.cpp | 25 +++++++++++- 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index 604a7cac0e4903..a45949314a4ca8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -256,13 +256,32 @@ void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) { .bind(CustomFunctionNamesId))) .bind(DeclRefId), this); + // C++ member calls do not contain a DeclRefExpr to the function decl. + // Instead, they contain a MemberExpr that refers to the decl. + Finder->addMatcher(memberExpr(member(functionDecl(CustomFunctionsMatcher) + .bind(CustomFunctionNamesId))) + .bind(DeclRefId), + this); } } void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { - const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId); - const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl()); - assert(DeclRef && FuncDecl && "No valid matched node in check()"); + const Expr *SourceExpr; + const FunctionDecl *FuncDecl; + + if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId)) { + SourceExpr = DeclRef; + FuncDecl = cast<FunctionDecl>(DeclRef->getDecl()); + } else if (const auto *Member = + Result.Nodes.getNodeAs<MemberExpr>(DeclRefId)) { + SourceExpr = Member; + FuncDecl = cast<FunctionDecl>(Member->getMemberDecl()); + } else { + llvm_unreachable("No valid matched node in check()"); + return; + } + + assert(SourceExpr && FuncDecl && "No valid matched node in check()"); // Only one of these are matched at a time. const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>( @@ -286,14 +305,15 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str(); if (Entry.Replacement.empty()) { - diag(DeclRef->getExprLoc(), "function %0 %1; it should not be used") + diag(SourceExpr->getExprLoc(), + "function %0 %1; it should not be used") << FuncDecl << Reason << Entry.Replacement - << DeclRef->getSourceRange(); + << SourceExpr->getSourceRange(); } else { - diag(DeclRef->getExprLoc(), + diag(SourceExpr->getExprLoc(), "function %0 %1; '%2' should be used instead") << FuncDecl << Reason << Entry.Replacement - << DeclRef->getSourceRange(); + << SourceExpr->getSourceRange(); } return; @@ -323,9 +343,9 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { if (!ReplacementFunctionName) return; - diag(DeclRef->getExprLoc(), "function %0 %1; '%2' should be used instead") + diag(SourceExpr->getExprLoc(), "function %0 %1; '%2' should be used instead") << FuncDecl << getRationaleFor(FunctionName) - << ReplacementFunctionName.value() << DeclRef->getSourceRange(); + << ReplacementFunctionName.value() << SourceExpr->getSourceRange(); } void UnsafeFunctionsCheck::registerPPCallbacks( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f967dfabd1c940..94f70c51f00a81 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -192,7 +192,7 @@ Changes in existing checks - Improved :doc:`bugprone-unsafe-functions <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying - additional functions to match. + additional functions to match, including C++ member functions. - Improved :doc:`bugprone-use-after-move <clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp index fc97d1bc93bc54..b707e471ef7cc4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp @@ -1,11 +1,16 @@ // RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\ -// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix'}}" +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix;^::S::member_match_,,is matched on a C++ class member'}}" // RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\ -// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match'}}" +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match;^::S::member_match_1$,,is matched on a C++ class member'}}" void name_match(); void prefix_match(); +struct S { + static void member_match_1() {} + void member_match_2() {} +}; + namespace regex_test { void name_match(); void prefix_match(); @@ -42,3 +47,19 @@ void f1() { // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used // no-warning STRICT-REGEX } + +void f2() { + S s; + + S::member_match_1(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + + s.member_match_1(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + + s.member_match_2(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_2' is matched on a C++ class member; it should not be used + // no-warning STRICT-REGEX +} >From 3bec1acf2c52142442a32564a7d9af5ce125bc76 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Thu, 5 Dec 2024 13:26:21 +0000 Subject: [PATCH 2/2] Add option to show fully qualified names of functions --- .../bugprone/UnsafeFunctionsCheck.cpp | 12 ++++ .../bugprone/UnsafeFunctionsCheck.h | 5 +- .../checks/bugprone/unsafe-functions.rst | 9 +++ .../unsafe-functions-custom-regex.cpp | 55 +++++++++++++------ 4 files changed, 62 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index a45949314a4ca8..ce4095e3f89163 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -21,6 +21,8 @@ namespace clang::tidy::bugprone { static constexpr llvm::StringLiteral OptionNameCustomFunctions = "CustomFunctions"; +static constexpr llvm::StringLiteral OptionNameShowFullyQualifiedNames = + "ShowFullyQualifiedNames"; static constexpr llvm::StringLiteral OptionNameReportDefaultFunctions = "ReportDefaultFunctions"; static constexpr llvm::StringLiteral OptionNameReportMoreUnsafeFunctions = @@ -185,6 +187,8 @@ UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name, : ClangTidyCheck(Name, Context), CustomFunctions(parseCheckedFunctions( Options.get(OptionNameCustomFunctions, ""), Context)), + ShowFullyQualifiedNames( + Options.get(OptionNameShowFullyQualifiedNames, false)), ReportDefaultFunctions( Options.get(OptionNameReportDefaultFunctions, true)), ReportMoreUnsafeFunctions( @@ -193,6 +197,8 @@ UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name, void UnsafeFunctionsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, OptionNameCustomFunctions, serializeCheckedFunctions(CustomFunctions)); + Options.store(Opts, OptionNameShowFullyQualifiedNames, + ShowFullyQualifiedNames); Options.store(Opts, OptionNameReportDefaultFunctions, ReportDefaultFunctions); Options.store(Opts, OptionNameReportMoreUnsafeFunctions, ReportMoreUnsafeFunctions); @@ -316,6 +322,12 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { << SourceExpr->getSourceRange(); } + if (ShowFullyQualifiedNames) { + diag(SourceExpr->getExprLoc(), + "fully qualified name of function is: '%0'", DiagnosticIDs::Note) + << FuncDecl->getQualifiedNameAsString(); + } + return; } } diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h index 63058c326ef291..869098117fb9a9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h @@ -43,7 +43,10 @@ class UnsafeFunctionsCheck : public ClangTidyCheck { private: const std::vector<CheckedFunction> CustomFunctions; - // If true, the default set of functions are reported. + /// If true, the fully qualified name of custom functions will be shown in a + /// note tag. + const bool ShowFullyQualifiedNames; + /// If true, the default set of functions are reported. const bool ReportDefaultFunctions; /// If true, additional functions from widely used API-s (such as POSIX) are /// added to the list of reported functions. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst index fb070627e31b1d..13e83a56c0fe51 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst @@ -114,6 +114,9 @@ qualified name (i.e. ``std::original``), otherwise the regex is matched against If the regular expression starts with `::` (or `^::`), it is matched against the fully qualified name (``::std::original``). +To aid with finding the fully qualified names of expressions, the option ``ShowFullyQualifiedNames`` can be used. +This option will show the fully qualified name of all functions matched by the custom function matchers. + Options ------- @@ -139,6 +142,12 @@ Options function, and an optional reason, separated by comma. For more information, see :ref:`Custom functions<CustomFunctions>`. +.. option:: ShowFullyQualifiedNames + + When `true`, the fully qualified name of all functions matched by the custom + function matchers will be shown. + Default is `false`. + Examples -------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp index b707e471ef7cc4..a208ebed72913b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp @@ -1,5 +1,5 @@ // RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\ -// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix;^::S::member_match_,,is matched on a C++ class member'}}" +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix;^::S::member_match_,,is matched on a C++ class member', bugprone-unsafe-functions.ShowFullyQualifiedNames: true}}" // RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\ // RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match;^::S::member_match_1$,,is matched on a C++ class member'}}" @@ -11,6 +11,9 @@ struct S { void member_match_2() {} }; +void member_match_1() {} +void member_match_unmatched() {} + namespace regex_test { void name_match(); void prefix_match(); @@ -21,30 +24,37 @@ void prefix_match_regex(); void f1() { name_match(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'name_match' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead prefix_match(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'prefix_match' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used ::name_match(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'name_match' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead regex_test::name_match(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'regex_test::name_match' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead name_match_regex(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match; 'replacement' should be used instead + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'name_match_regex' // no-warning STRICT-REGEX ::prefix_match(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'prefix_match' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used regex_test::prefix_match(); // no-warning NON-STRICT-REGEX // no-warning STRICT-REGEX prefix_match_regex(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'prefix_match_regex' // no-warning STRICT-REGEX } @@ -52,14 +62,23 @@ void f2() { S s; S::member_match_1(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:3: note: fully qualified name of function is: 'S::member_match_1' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used s.member_match_1(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used - // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:5: note: fully qualified name of function is: 'S::member_match_1' + // CHECK-NOTES-STRICT-REGEX: :[[@LINE-3]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used s.member_match_2(); - // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_2' is matched on a C++ class member; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_2' is matched on a C++ class member; it should not be used + // CHECK-NOTES-NON-STRICT-REGEX: :[[@LINE-2]]:5: note: fully qualified name of function is: 'S::member_match_2' // no-warning STRICT-REGEX + + member_match_1(); + // no-warning + + member_match_unmatched(); + // no-warning } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits