Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
38 changes: 29 additions & 9 deletions clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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
}
Loading