Author: Discookie Date: 2025-11-18T15:26:20Z New Revision: 40645ed4ed7ce853d9cc76bcc4aeabb6a83a0f2c
URL: https://github.com/llvm/llvm-project/commit/40645ed4ed7ce853d9cc76bcc4aeabb6a83a0f2c DIFF: https://github.com/llvm/llvm-project/commit/40645ed4ed7ce853d9cc76bcc4aeabb6a83a0f2c.diff LOG: [clang-tidy] Add a fully custom message to `bugprone-unsafe-functions` (#162443) In some cases, such as when recommending the compiler option _FORTIFY_SOURCE, the current custom message format is clunky. Now, when the reason starts with `>`, the replacement string is omitted., so only the Reason is shown. `^function$,,has a custom message;` - function 'function' has a custom message; it should not be used `^function$,,>has a custom message and no replacement suggestion;` - function 'function' has a custom message and no replacement suggestion --------- Co-authored-by: Donát Nagy <[email protected]> Added: Modified: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index 5524c4b484be1..67d0931003c54 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -301,14 +301,20 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { if (Custom) { for (const auto &Entry : CustomFunctions) { if (Entry.Pattern.match(*FuncDecl)) { - const StringRef Reason = + StringRef Reason = Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str(); - if (Entry.Replacement.empty()) { + // Omit the replacement, when a fully-custom reason is given. + if (Reason.consume_front(">")) { + diag(SourceExpr->getExprLoc(), "function %0 %1") + << FuncDecl << Reason.trim() << SourceExpr->getSourceRange(); + // Do not recommend a replacement when it is not present. + } else if (Entry.Replacement.empty()) { diag(SourceExpr->getExprLoc(), "function %0 %1; it should not be used") << FuncDecl << Reason << Entry.Replacement << SourceExpr->getSourceRange(); + // Otherwise, emit the replacement. } else { diag(SourceExpr->getExprLoc(), "function %0 %1; '%2' should be used instead") diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b982216297919..743397e3ec6ce 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -69,6 +69,13 @@ Potentially Breaking Changes - `CharTypdefsToIgnore` to `CharTypedefsToIgnore` in :doc:`bugprone-signed-char-misuse <clang-tidy/checks/bugprone/signed-char-misuse>` + +- Modified the custom message format of :doc:`bugprone-unsafe-functions + <clang-tidy/checks/bugprone/unsafe-functions>` by assigning a special meaning + to the character ``>`` at the start of the value of the option + ``CustomFunctions``. If the option value starts with ``>``, then the + replacement suggestion part of the message (which would be included by + default) is omitted. (This does not change the warning locations.) - :program:`clang-tidy` now displays warnings from all non-system headers by default. Previously, users had to explicitly opt-in to header warnings using @@ -387,6 +394,11 @@ Changes in existing checks <clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding an additional matcher that generalizes the copy-and-swap idiom pattern detection. + +- Improved :doc:`bugprone-unsafe-functions + <clang-tidy/checks/bugprone/unsafe-functions>` check by hiding the default + suffix when the reason starts with the character `>` in the `CustomFunctions` + option. - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check 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 f1fec13739271..cb7ea415c54b2 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 @@ -96,37 +96,62 @@ to be checked. The format is the following, without newlines: The functions are matched using POSIX extended regular expressions. *(Note: The regular expressions do not support negative* ``(?!)`` *matches.)* -The `reason` is optional and is used to provide additional information -about the reasoning behind the replacement. The default reason is -`is marked as unsafe`. +The ``reason`` is optional and is used to provide additional information about the +reasoning behind the replacement. The default reason is ``is marked as unsafe``. -If `replacement` is empty, the text `it should not be used` will be shown -instead of the suggestion for a replacement. +If ``replacement`` is empty, the default text ``it should not be used`` will be +shown instead of the suggestion for a replacement. -As an example, the configuration `^original$, replacement, is deprecated;` -will produce the following diagnostic message. +If the ``reason`` starts with the character ``>``, the reason becomes fully custom. +The default suffix is disabled even if a ``replacement`` is present, and only the +reason message is shown after the matched function, to allow better control over +the suggestions. (The starting ``>`` and whitespace directly after it are +trimmed from the message.) + +As an example, the following configuration matches only the function ``original`` +in the default namespace. A similar diagnostic can also be printed using a fully +custom reason. .. code:: c + // bugprone-unsafe-functions.CustomFunctions: + // ^original$, replacement, is deprecated; + // Using the fully custom message syntax: + // ^suspicious$,,> should be avoided if possible. original(); // warning: function 'original' is deprecated; 'replacement' should be used instead. + suspicious(); // warning: function 'suspicious' should be avoided if possible. ::std::original(); // no-warning original_function(); // no-warning -If the regular expression contains the character `:`, it is matched against the -qualified name (i.e. ``std::original``), otherwise the regex is matched against the unqualified name (``original``). -If the regular expression starts with `::` (or `^::`), it is matched against the -fully qualified name (``::std::original``). +If the regular expression contains the character ``:``, it is matched against the +qualified name (i.e. ``std::original``), otherwise the regex is matched against +the unqualified name (``original``). If the regular expression starts with ``::`` +(or ``^::``), it is matched against the fully qualified name +(``::std::original``). + +One of the use cases for fully custom messages is suggesting compiler options +and warning flags: + +.. code:: c + + // bugprone-unsafe-functions.CustomFunctions: + // ^memcpy$,,>is recommended to have compiler hardening using '_FORTIFY_SOURCE'; + // ^printf$,,>is recommended to have the '-Werror=format-security' compiler warning flag; + + memcpy(dest, src, 999'999); // warning: function 'memcpy' is recommended to have compiler hardening using '_FORTIFY_SOURCE' + printf(raw_str); // warning: function 'printf' is recommended to have the '-Werror=format-security' compiler warning flag .. note:: - Fully qualified names can contain template parameters on certain C++ classes, but not on C++ functions. - Type aliases are resolved before matching. + Fully qualified names can contain template parameters on certain C++ classes, + but not on C++ functions. Type aliases are resolved before matching. As an example, the member function ``open`` in the class ``std::ifstream`` has a fully qualified name of ``::std::basic_ifstream<char>::open``. - The example could also be matched with the regex ``::std::basic_ifstream<[^>]*>::open``, which matches all potential - template parameters, but does not match nested template classes. + The example could also be matched with the regex + ``::std::basic_ifstream<[^>]*>::open``, which matches all potential template + parameters, but does not match nested template classes. Options ------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c index 7fd71ec2f2e7b..7eaf015f06aa2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c @@ -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'}}" +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: \"::name_match,,>is a qualname match, but with a fully 'custom' message;^::prefix_match,,is matched on qualname prefix\"}}" // 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'}}" @@ -11,14 +11,14 @@ 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-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match, but with a fully 'custom' message // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]: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 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-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match, but with a fully 'custom' message // no-warning STRICT-REGEX prefix_match_regex(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
