llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Discookie (Discookie) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/162443.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp (+7-1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst (+5-1) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c (+3-3) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index 0399af2a673f4..2470ea9681e9e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -304,11 +304,17 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { 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 d13f83079e591..c876aaf1d182a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -270,6 +270,11 @@ Changes in existing checks 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 omitting the custom + replacement string, when the Reason starts with the character `>` in the + `CustomFunctions` option. + - Improved :doc:`cppcoreguidelines-prefer-member-initializer <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to avoid false positives on inherited members in class templates. 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 317db9c5564e2..8945d3e91e631 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 @@ -98,7 +98,9 @@ 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. +instead of the suggestion for a replacement. If the `reason` starts with the +character `>`, the replacement message is disabled, to allow better control over +the suggestions. As an example, the configuration `^original$, replacement, is deprecated;` will produce the following diagnostic message. @@ -114,6 +116,8 @@ 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``). +The same diagnostic message is produced by the configuration `^original$,,> is deprecated; 'replacement' should be used instead` + .. note:: Fully qualified names can contain template parameters on certain C++ classes, but not on C++ functions. 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(); `````````` </details> https://github.com/llvm/llvm-project/pull/162443 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
