Skip to content

Commit 476d571

Browse files
committed
[clang-tidy] Add regex support and documentation for C++ functions for bugprone-unsafe-functions check
1 parent 47c1a56 commit 476d571

File tree

5 files changed

+76
-8
lines changed

5 files changed

+76
-8
lines changed

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,10 @@ ParseCheckedFunctions(StringRef Option, StringRef OptionName,
167167
<< Name.trim() << OptionName;
168168
}
169169

170-
Result.push_back({Name.trim().str(), llvm::Regex(Name.trim()),
171-
Replacement.trim().str(), Reason.trim().str()});
170+
Result.push_back(
171+
{Name.trim().str(),
172+
matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()),
173+
Replacement.trim().str(), Reason.trim().str()});
172174
}
173175

174176
return Result;
@@ -324,7 +326,7 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
324326

325327
if (AnnexKIsAvailable) {
326328
for (const auto &Entry : CustomAnnexKFunctions) {
327-
if (Entry.Pattern.match(FunctionName)) {
329+
if (Entry.Pattern.match(*FuncDecl)) {
328330
// If both Annex K and Normal are matched, show Annex K warning only.
329331
if (CustomAnnexK)
330332
ShowCheckedFunctionWarning(Entry);
@@ -341,7 +343,8 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
341343
return;
342344

343345
for (const auto &Entry : CustomNormalFunctions) {
344-
if (Entry.Pattern.match(FunctionName)) {
346+
347+
if (Entry.Pattern.match(*FuncDecl)) {
345348
ShowCheckedFunctionWarning(Entry);
346349
return;
347350
}

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFUNCTIONSCHECK_H
1111

1212
#include "../ClangTidyCheck.h"
13+
#include "../utils/Matchers.h"
1314
#include <optional>
1415

1516
namespace clang::tidy::bugprone {
@@ -34,7 +35,7 @@ class UnsafeFunctionsCheck : public ClangTidyCheck {
3435

3536
struct CheckedFunction {
3637
std::string Name;
37-
llvm::Regex Pattern;
38+
matchers::MatchesAnyListedNameMatcher::NameMatcher Pattern;
3839
std::string Replacement;
3940
std::string Reason;
4041
};

clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ will produce the following diagnostic message.
107107
::std::original(); // no-warning
108108
original_function(); // no-warning
109109
110+
If the regular expression contains the character ``:``, it is matched against the
111+
qualified name (i.e. ``std::original``), otherwise the regex is matched against the unqualified name (``original``).
112+
If the regular expression starts with ``::`` (or ``^::``), it is matched against the
113+
fully qualified name (``::std::original``).
110114

111115
Options
112116
-------
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// This test fails on "x86_64-sie" buildbot and "x86_64-scei-ps4" target.
2+
// According to @dyung, something related to the kind of standard library
3+
// availability is causing the failure. Even though we explicitly define
4+
// the relevant macros the check is hunting for in the invocation, the real
5+
// parsing and preprocessor state will not have that case.
6+
// UNSUPPORTED: target={{.*-(ps4|ps5)}}
7+
//
8+
// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\
9+
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomNormalFunctions: '::non_annex_k_only,replacement;::both,replacement,is a normal function'}}"\
10+
// RUN: -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__
11+
// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\
12+
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomNormalFunctions: '^non_annex_k_only$,replacement,is matched on function name only;^::both,replacement,is matched on qualname prefix'}}"\
13+
// RUN: -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__
14+
// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K %s bugprone-unsafe-functions %t --\
15+
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomAnnexKFunctions: '::annex_k_only,replacement;::both,replacement,is an Annex K function'}}"\
16+
// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
17+
18+
// TODO: How is "Annex K" available in C++ mode?
19+
// no-warning WITH-ANNEX-K
20+
21+
void non_annex_k_only();
22+
void annex_k_only();
23+
void both();
24+
25+
namespace regex_test {
26+
void non_annex_k_only();
27+
void both();
28+
}
29+
30+
void non_annex_k_only_regex();
31+
void both_regex();
32+
33+
void f1() {
34+
non_annex_k_only();
35+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead
36+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is matched on function name only; 'replacement' should be used instead
37+
annex_k_only();
38+
// no-warning NON-STRICT-REGEX
39+
// no-warning STRICT-REGEX
40+
both();
41+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead
42+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'both' is matched on qualname prefix; 'replacement' should be used instead
43+
44+
::non_annex_k_only();
45+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead
46+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is matched on function name only; 'replacement' should be used instead
47+
regex_test::non_annex_k_only();
48+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead
49+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is matched on function name only; 'replacement' should be used instead
50+
non_annex_k_only_regex();
51+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only_regex' is marked as unsafe; 'replacement' should be used instead
52+
// no-warning STRICT-REGEX
53+
54+
::both();
55+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead
56+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'both' is matched on qualname prefix; 'replacement' should be used instead
57+
regex_test::both();
58+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead
59+
// no-warning STRICT-REGEX
60+
both_regex();
61+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both_regex' is a normal function; 'replacement' should be used instead
62+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'both_regex' is matched on qualname prefix; 'replacement' should be used instead
63+
}

clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomAnnexKFunctions: '^annex_k_only$,replacement;^both$,replacement,is an Annex K function'}}"\
1616
// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1
1717

18-
typedef __SIZE_TYPE__ size_t;
19-
typedef __WCHAR_TYPE__ wchar_t;
20-
2118
void non_annex_k_only(void);
2219
void annex_k_only(void);
2320
void both(void);

0 commit comments

Comments
 (0)