[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.

Author: PiotrZSL

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);

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

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;

clang-tools-extra/docs/ReleaseNotes.rst

@@ -235,6 +235,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

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.

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>`_.

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() {

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 {