[PATCH] D151383: [clang-tidy] Check for specific return types on all functions

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL requested changes to this revision.
PiotrZSL added a comment.
This revision now requires changes to proceed.

- Missing tests
- Missing release notes
- Missing check documentation update



================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:159
 
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(anyOf(
----------------
This may cause check method to be executed twice for same AST node.
Instead of adding new matcher you should just extend MatchedCallExpr to include all methods that `isInstantiatedFrom(hasAnyName(FunVec))` or returns one of those specific types.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:161
+      callExpr(callee(functionDecl(anyOf(
+                   hasReturnTypeLoc(loc(asString("std::error_code"))),
+                   hasReturnTypeLoc(loc(asString("std::expected"))),
----------------
Put those return types into some configuration option instead of hardcoding it.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:162
+                   hasReturnTypeLoc(loc(asString("std::error_code"))),
+                   hasReturnTypeLoc(loc(asString("std::expected"))),
+                   hasReturnTypeLoc(loc(asString("boost::system::error_code"))),
----------------
check if this work with typedefs/aliases...


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:176-177
 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")
-        << Matched->getSourceRange();
-    diag(Matched->getBeginLoc(),
-         "cast the expression to void to silence this warning",
-         DiagnosticIDs::Note);
+  const char *callExprBindingNames[] = {"return-types", "match"};
+  for (const char *callExprBindingName : callExprBindingNames) {
+    if (const auto *Matched =
----------------
this `for` is not needed, just use same "bind" name, and you wont need to change this method at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151383/new/

https://reviews.llvm.org/D151383