[clang-tools-extra] [clang-tidy] Add C++ member function support to custom bugprone-unsafe-functions matches (PR #117165)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 5 05:26:38 PST 2024
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.cseh at 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.cseh at 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
}
More information about the cfe-commits
mailing list