[clang-tools-extra] [clang-tidy] Improve bugprone-unused-return-value check (PR #66573)
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 16 05:06:37 PDT 2023
https://github.com/PiotrZSL created https://github.com/llvm/llvm-project/pull/66573
Improve diagnostic message to be more straight forward, fix handling of
casting to non-void and add new option AllowCastToVoid to control casting
to void behavior.
Closes #66570
>From b1ab97c39754126d66ed40fc6d31ee54ac96b1a7 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sat, 16 Sep 2023 08:46:02 +0000
Subject: [PATCH 1/3] [clang-tidy] Improve bugprone-unused-return-value
diagnostic
Improve diagnostic message to be more straight forward.
---
.../bugprone/UnusedReturnValueCheck.cpp | 3 +-
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
.../bugprone/unused-return-value-custom.cpp | 32 +++----
.../checkers/bugprone/unused-return-value.cpp | 96 +++++++++----------
.../test/clang-tidy/checkers/cert/err33-c.c | 12 +--
5 files changed, 75 insertions(+), 71 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index bdc601c2445f5e6..791385fbb18b493 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -178,7 +178,8 @@ void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Matched = Result.Nodes.getNodeAs<CallExpr>("match")) {
diag(Matched->getBeginLoc(),
- "the value returned by this function should be used")
+ "the value returned by this function should not be disregarded; "
+ "neglecting it may lead to errors")
<< Matched->getSourceRange();
diag(Matched->getBeginLoc(),
"cast the expression to void to silence this warning",
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d6f51998a01e57..4230d62dfaae407 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -202,6 +202,9 @@ Changes in existing checks
<clang-tidy/checks/bugprone/reserved-identifier>` check, so that it does not
warn on macros starting with underscore and lowercase letter.
+- Improved :doc:`bugprone-unused-return-value
+ <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message.
+
- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
<clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
to ignore ``static`` variables declared within the scope of
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
index fb5f77031a3276f..a949fb37884b880 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
@@ -47,40 +47,40 @@ void fun(int);
void warning() {
fun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
(fun());
- // CHECK-NOTES: [[@LINE-1]]:4: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:4: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:4: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:4: note: cast the expression to void to silence this warning
ns::Outer::Inner ObjA1;
ObjA1.memFun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::AliasName::Inner ObjA2;
ObjA2.memFun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::Derived ObjA3;
ObjA3.memFun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::Type::staticFun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::ClassTemplate<int> ObjA4;
ObjA4.memFun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::ClassTemplate<int>::staticFun();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
}
void noWarning() {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index 6da66ca42d90fa8..db33edc370a2309 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -81,121 +81,121 @@ std::error_code errorFunc() {
void warning() {
std::async(increment, 42);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::async(std::launch::async, increment, 42);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
Foo F;
std::launder(&F);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::remove_if(nullptr, nullptr, nullptr);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::unique(nullptr, nullptr);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::unique_ptr<Foo> UPtr;
UPtr.release();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::string Str;
Str.empty();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
std::vector<Foo> Vec;
Vec.empty();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
// test discarding return values inside different kinds of statements
auto Lambda = [] { std::remove(nullptr, nullptr, 1); };
- // CHECK-NOTES: [[@LINE-1]]:22: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:22: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:22: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:22: note: cast the expression to void to silence this warning
if (true)
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
else if (true)
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
else
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
while (true)
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
do
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
while (true);
for (;;)
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
for (std::remove(nullptr, nullptr, 1);;)
- // CHECK-NOTES: [[@LINE-1]]:8: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:8: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:8: note: cast the expression to void to silence this warning
;
for (;; std::remove(nullptr, nullptr, 1))
- // CHECK-NOTES: [[@LINE-1]]:11: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:11: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:11: note: cast the expression to void to silence this warning
;
for (auto C : "foo")
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
switch (1) {
case 1:
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
break;
default:
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
break;
}
try {
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
} catch (...) {
std::remove(nullptr, nullptr, 1);
- // CHECK-NOTES: [[@LINE-1]]:5: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:5: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:5: note: cast the expression to void to silence this warning
}
errorFunc();
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value {{.*}} should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast {{.*}} this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
}
void noWarning() {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c b/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c
index 520ff17bd890eae..87ce0acf664e6d4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert/err33-c.c
@@ -4,15 +4,15 @@ typedef __SIZE_TYPE__ size_t;
void *aligned_alloc(size_t alignment, size_t size);
void test_aligned_alloc(void) {
aligned_alloc(2, 10);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
}
long strtol(const char *restrict nptr, char **restrict endptr, int base);
void test_strtol(void) {
strtol("123", 0, 10);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
}
typedef char wchar_t;
@@ -20,6 +20,6 @@ int wscanf_s(const wchar_t *restrict format, ...);
void test_wscanf_s(void) {
int Val;
wscanf_s("%i", &Val);
- // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
- // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
}
>From a35cbbc96e6974aff521180cd5aa3308f82c8c15 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sat, 16 Sep 2023 11:26:45 +0000
Subject: [PATCH 2/3] [clang-tidy] Add support for explicit casts in
bugprone-unused-return-value
Issues could be silenced by for example casting
bool result value to int, now it won't work, and
problem will be detected.
---
.../bugprone/UnusedReturnValueCheck.cpp | 24 ++++++++++++-------
.../bugprone/UnusedReturnValueCheck.h | 3 +++
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++-
.../checkers/bugprone/unused-return-value.cpp | 4 ++++
4 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 791385fbb18b493..4a499c69c294e37 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -141,15 +141,21 @@ void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
auto FunVec = utils::options::parseStringList(CheckedFunctions);
- auto MatchedCallExpr = expr(ignoringImplicit(ignoringParenImpCasts(
- callExpr(callee(functionDecl(
- // Don't match void overloads of checked functions.
- unless(returns(voidType())),
- anyOf(isInstantiatedFrom(hasAnyName(FunVec)),
- returns(hasCanonicalType(hasDeclaration(
- namedDecl(matchers::matchesAnyListedName(
- CheckedReturnTypes)))))))))
- .bind("match"))));
+ auto MatchedDirectCallExpr =
+ expr(callExpr(callee(functionDecl(
+ // Don't match void overloads of checked functions.
+ unless(returns(voidType())),
+ anyOf(isInstantiatedFrom(hasAnyName(FunVec)),
+ returns(hasCanonicalType(hasDeclaration(
+ namedDecl(matchers::matchesAnyListedName(
+ CheckedReturnTypes)))))))))
+ .bind("match"));
+
+ auto MatchedCallExpr =
+ expr(anyOf(MatchedDirectCallExpr,
+ explicitCastExpr(unless(cxxFunctionalCastExpr()),
+ unless(hasCastKind(CK_ToVoid)),
+ hasSourceExpression(MatchedDirectCallExpr))));
auto UnusedInCompoundStmt =
compoundStmt(forEach(MatchedCallExpr),
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index 15881e12ca0c36a..f4288c0e72f2db4 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -24,6 +24,9 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
private:
std::string CheckedFunctions;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 4230d62dfaae407..fbb5d42d3c360f4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -203,7 +203,8 @@ Changes in existing checks
warn on macros starting with underscore and lowercase letter.
- Improved :doc:`bugprone-unused-return-value
- <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message.
+ <clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message,
+ added support for detection of unused results when cast to non-``void`` type.
- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
<clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index db33edc370a2309..1e2f67367f448bc 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -115,6 +115,10 @@ void warning() {
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
// CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+ (int)Str.empty();
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
+ // CHECK-MESSAGES: [[@LINE-2]]:8: note: cast the expression to void to silence this warning
+
std::vector<Foo> Vec;
Vec.empty();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
>From be2cd0a17d65b5cad7d66bf8e75cd87a2165e459 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <piotr.zegar at nokia.com>
Date: Sat, 16 Sep 2023 11:56:26 +0000
Subject: [PATCH 3/3] [clang-tidy] Add AllowCastToVoid option to
bugprone-unused-return-value
Add option that control casting to void behavior,
change default issue suppression to disallow casting
to void.
---
.../bugprone/UnusedReturnValueCheck.cpp | 19 +++++++++++++------
.../bugprone/UnusedReturnValueCheck.h | 1 +
.../clang-tidy/cert/CERTTidyModule.cpp | 1 +
clang-tools-extra/docs/ReleaseNotes.rst | 2 ++
.../checks/bugprone/unused-return-value.rst | 4 ++++
.../docs/clang-tidy/checks/cert/err33-c.rst | 3 +++
.../bugprone/unused-return-value-custom.cpp | 11 +++--------
.../checkers/bugprone/unused-return-value.cpp | 3 ++-
8 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 4a499c69c294e37..6bc9f2dd367dcbd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -130,12 +130,14 @@ UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
"::std::error_condition;"
"::std::errc;"
"::std::expected;"
- "::boost::system::error_code"))) {}
+ "::boost::system::error_code"))),
+ AllowCastToVoid(Options.get("AllowCastToVoid", false)) {}
void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CheckedFunctions", CheckedFunctions);
Options.store(Opts, "CheckedReturnTypes",
utils::options::serializeStringList(CheckedReturnTypes));
+ Options.store(Opts, "AllowCastToVoid", AllowCastToVoid);
}
void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
@@ -151,11 +153,12 @@ void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) {
CheckedReturnTypes)))))))))
.bind("match"));
- auto MatchedCallExpr =
- expr(anyOf(MatchedDirectCallExpr,
- explicitCastExpr(unless(cxxFunctionalCastExpr()),
- unless(hasCastKind(CK_ToVoid)),
- hasSourceExpression(MatchedDirectCallExpr))));
+ auto CheckCastToVoid =
+ AllowCastToVoid ? castExpr(unless(hasCastKind(CK_ToVoid))) : castExpr();
+ auto MatchedCallExpr = expr(
+ anyOf(MatchedDirectCallExpr,
+ explicitCastExpr(unless(cxxFunctionalCastExpr()), CheckCastToVoid,
+ hasSourceExpression(MatchedDirectCallExpr))));
auto UnusedInCompoundStmt =
compoundStmt(forEach(MatchedCallExpr),
@@ -187,6 +190,10 @@ void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
"the value returned by this function should not be disregarded; "
"neglecting it may lead to errors")
<< Matched->getSourceRange();
+
+ if (!AllowCastToVoid)
+ return;
+
diag(Matched->getBeginLoc(),
"cast the expression to void to silence this warning",
DiagnosticIDs::Note);
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
index f4288c0e72f2db4..b4356f8379fdc85 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -31,6 +31,7 @@ class UnusedReturnValueCheck : public ClangTidyCheck {
private:
std::string CheckedFunctions;
const std::vector<StringRef> CheckedReturnTypes;
+ const bool AllowCastToVoid;
};
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index d448d9ba6145481..d334ab8c437d32a 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -328,6 +328,7 @@ class CERTModule : public ClangTidyModule {
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
Opts["cert-err33-c.CheckedFunctions"] = CertErr33CCheckedFunctions;
+ Opts["cert-err33-c.AllowCastToVoid"] = "true";
Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "false";
Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "false";
return Options;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fbb5d42d3c360f4..7432333024f44fe 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -205,6 +205,8 @@ Changes in existing checks
- Improved :doc:`bugprone-unused-return-value
<clang-tidy/checks/bugprone/unused-return-value>` check diagnostic message,
added support for detection of unused results when cast to non-``void`` type.
+ Casting to ``void`` no longer suppresses issues by default, control this
+ behavior with the new `AllowCastToVoid` 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/unused-return-value.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
index 5ca96216a10043e..1e3c8a3268222ed 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unused-return-value.rst
@@ -52,5 +52,9 @@ Options
By default the following function return types are checked:
`::std::error_code`, `::std::error_condition`, `::std::errc`, `::std::expected`, `::boost::system::error_code`
+.. option:: AllowCastToVoid
+
+ Controls whether casting return values to ``void`` is permitted. Default: `false`.
+
:doc:`cert-err33-c <../cert/err33-c>` is an alias of this check that checks a
fixed and large set of standard library functions.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst
index f3211e50b7e6cad..c1414ec5e086db1 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/err33-c.rst
@@ -189,6 +189,9 @@ functions are checked:
This check is an alias of check :doc:`bugprone-unused-return-value <../bugprone/unused-return-value>`
with a fixed set of functions.
+Suppressing issues by casting to ``void`` is enabled by default and can be
+disabled by setting `AllowCastToVoid` option to ``false``.
+
The check corresponds to a part of CERT C Coding Standard rule `ERR33-C.
Detect and handle standard library errors
<https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors>`_.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
index a949fb37884b880..d3650b210ab02b5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value-custom.cpp
@@ -48,39 +48,34 @@ void fun(int);
void warning() {
fun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
(fun());
// CHECK-MESSAGES: [[@LINE-1]]:4: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:4: note: cast the expression to void to silence this warning
+
+ (void)fun();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
ns::Outer::Inner ObjA1;
ObjA1.memFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::AliasName::Inner ObjA2;
ObjA2.memFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::Derived ObjA3;
ObjA3.memFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::Type::staticFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::ClassTemplate<int> ObjA4;
ObjA4.memFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
ns::ClassTemplate<int>::staticFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should not be disregarded; neglecting it may lead to errors
- // CHECK-MESSAGES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
}
void noWarning() {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
index 1e2f67367f448bc..5c6ce1e4bf1fd21 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-return-value.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s bugprone-unused-return-value %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t -- \
+// RUN: --config="{CheckOptions: {bugprone-unused-return-value.AllowCastToVoid: true}}" -- -fexceptions
namespace std {
More information about the cfe-commits
mailing list