[clang-tools-extra] r327833 - [clang-tidy] New check bugprone-unused-return-value

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 19 06:02:33 PDT 2018


Author: alexfh
Date: Mon Mar 19 06:02:32 2018
New Revision: 327833

URL: http://llvm.org/viewvc/llvm-project?rev=327833&view=rev
Log:
[clang-tidy] New check bugprone-unused-return-value

Summary:
Detects function calls where the return value is unused.

Checked functions can be configured.

Reviewers: alexfh, aaron.ballman, ilya-biryukov, hokein

Reviewed By: alexfh, aaron.ballman

Subscribers: hintonda, JonasToth, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Patch by Kalle Huttunen!

Differential Revision: https://reviews.llvm.org/D41655

Added:
    clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
    clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst
    clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp
    clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
    clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
    clang-tools-extra/trunk/docs/ReleaseNotes.rst
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=327833&r1=327832&r2=327833&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Mon Mar 19 06:02:32 2018
@@ -43,6 +43,7 @@
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
 #include "UnusedRaiiCheck.h"
+#include "UnusedReturnValueCheck.h"
 #include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
 
@@ -119,6 +120,8 @@ public:
         "bugprone-undelegated-constructor");
     CheckFactories.registerCheck<UnusedRaiiCheck>(
         "bugprone-unused-raii");
+    CheckFactories.registerCheck<UnusedReturnValueCheck>(
+        "bugprone-unused-return-value");
     CheckFactories.registerCheck<UseAfterMoveCheck>(
         "bugprone-use-after-move");
     CheckFactories.registerCheck<VirtualNearMissCheck>(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=327833&r1=327832&r2=327833&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Mon Mar 19 06:02:32 2018
@@ -35,6 +35,7 @@ add_clang_library(clangTidyBugproneModul
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
   UnusedRaiiCheck.cpp
+  UnusedReturnValueCheck.cpp
   UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 

Added: clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp?rev=327833&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp Mon Mar 19 06:02:32 2018
@@ -0,0 +1,82 @@
+//===--- UnusedReturnValueCheck.cpp - clang-tidy---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnusedReturnValueCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
+                                               ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      CheckedFunctions(Options.get("CheckedFunctions", "::std::async;"
+                                                       "::std::launder;"
+                                                       "::std::remove;"
+                                                       "::std::remove_if;"
+                                                       "::std::unique")) {}
+
+void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "CheckedFunctions", CheckedFunctions);
+}
+
+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())), hasAnyName(std::vector<StringRef>(
+                                               FunVec.begin(), FunVec.end())))))
+          .bind("match"))));
+
+  auto UnusedInCompoundStmt =
+      compoundStmt(forEach(MatchedCallExpr),
+                   // The checker can't currently differentiate between the
+                   // return statement and other statements inside GNU statement
+                   // expressions, so disable the checker inside them to avoid
+                   // false positives.
+                   unless(hasParent(stmtExpr())));
+  auto UnusedInIfStmt =
+      ifStmt(eachOf(hasThen(MatchedCallExpr), hasElse(MatchedCallExpr)));
+  auto UnusedInWhileStmt = whileStmt(hasBody(MatchedCallExpr));
+  auto UnusedInDoStmt = doStmt(hasBody(MatchedCallExpr));
+  auto UnusedInForStmt =
+      forStmt(eachOf(hasLoopInit(MatchedCallExpr),
+                     hasIncrement(MatchedCallExpr), hasBody(MatchedCallExpr)));
+  auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(MatchedCallExpr));
+  auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr));
+
+  Finder->addMatcher(
+      stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt,
+                 UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt,
+                 UnusedInCaseStmt)),
+      this);
+}
+
+void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Matched = Result.Nodes.getNodeAs<CallExpr>("match")) {
+    diag(Matched->getLocStart(),
+         "the value returned by this function should be used")
+        << Matched->getSourceRange();
+    diag(Matched->getLocStart(),
+         "cast the expression to void to silence this warning",
+         DiagnosticIDs::Note);
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h?rev=327833&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h Mon Mar 19 06:02:32 2018
@@ -0,0 +1,39 @@
+//===--- UnusedReturnValueCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNUSEDRETURNVALUECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNUSEDRETURNVALUECHECK_H
+
+#include "../ClangTidy.h"
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Detects function calls where the return value is unused.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unused-return-value.html
+class UnusedReturnValueCheck : public ClangTidyCheck {
+public:
+  UnusedReturnValueCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  std::string CheckedFunctions;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNUSEDRETURNVALUECHECK_H

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=327833&r1=327832&r2=327833&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Mon Mar 19 06:02:32 2018
@@ -67,6 +67,11 @@ Improvements to clang-tidy
   Diagnoses when a temporary object that appears to be an exception is
   constructed but not thrown.
 
+- New `bugprone-unused-return-value
+  <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unused-return-value.html>`_ check
+
+  Warns on unused function return values.
+
 - New `cppcoreguidelines-avoid-goto
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_ check
 

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst?rev=327833&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst Mon Mar 19 06:02:32 2018
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - bugprone-unused-return-value
+
+bugprone-unused-return-value
+============================
+
+Warns on unused function return values. The checked funtions can be configured.
+
+Options
+-------
+
+.. option:: CheckedFunctions
+
+   Semicolon-separated list of functions to check. Defaults to
+   ``::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique``.
+   This means that the calls to following functions are checked by default:
+
+   - ``std::async()``. Not using the return value makes the call synchronous.
+   - ``std::launder()``. Not using the return value usually means that the
+     function interface was misunderstood by the programmer. Only the returned
+     pointer is "laundered", not the argument.
+   - ``std::remove()``, ``std::remove_if()`` and ``std::unique()``. The returned
+     iterator indicates the boundary between elements to keep and elements to be
+     removed. Not using the return value means that the information about which
+     elements to remove is lost.

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=327833&r1=327832&r2=327833&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Mon Mar 19 06:02:32 2018
@@ -51,6 +51,7 @@ Clang-Tidy Checks
    bugprone-undefined-memory-manipulation
    bugprone-undelegated-constructor
    bugprone-unused-raii
+   bugprone-unused-return-value
    bugprone-use-after-move
    bugprone-virtual-near-miss
    cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>

Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp?rev=327833&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp Mon Mar 19 06:02:32 2018
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: bugprone-unused-return-value.CheckedFunctions, \
+// RUN:    value: "::fun;::ns::Outer::Inner::memFun;::ns::Type::staticFun"}]}' \
+// RUN: --
+
+namespace std {
+
+template <typename T>
+T *launder(T *);
+
+} // namespace std
+
+namespace ns {
+
+struct Outer {
+  struct Inner {
+    bool memFun();
+  };
+};
+
+using AliasName = Outer;
+
+struct Derived : public Outer::Inner {};
+
+struct Retval {
+  int *P;
+  Retval() { P = new int; }
+  ~Retval() { delete P; }
+};
+
+struct Type {
+  Retval memFun();
+  static Retval staticFun();
+};
+
+} // namespace ns
+
+int fun();
+void fun(int);
+
+void warning() {
+  fun();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  (fun());
+  // CHECK-MESSAGES: [[@LINE-1]]:4: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  ns::Outer::Inner ObjA1;
+  ObjA1.memFun();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  ns::AliasName::Inner ObjA2;
+  ObjA2.memFun();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  ns::Derived ObjA3;
+  ObjA3.memFun();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  ns::Type::staticFun();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+}
+
+void noWarning() {
+  auto R1 = fun();
+
+  ns::Outer::Inner ObjB1;
+  auto R2 = ObjB1.memFun();
+
+  auto R3 = ns::Type::staticFun();
+
+  // test calling a void overload of a checked function
+  fun(5);
+
+  // test discarding return value of functions that are not configured to be checked
+  int I = 1;
+  std::launder(&I);
+
+  ns::Type ObjB2;
+  ObjB2.memFun();
+}

Added: clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp?rev=327833&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp Mon Mar 19 06:02:32 2018
@@ -0,0 +1,166 @@
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t
+
+namespace std {
+
+struct future {};
+
+enum class launch {
+  async,
+  deferred
+};
+
+template <typename Function, typename... Args>
+future async(Function &&, Args &&...);
+
+template <typename Function, typename... Args>
+future async(launch, Function &&, Args &&...);
+
+template <typename ForwardIt, typename T>
+ForwardIt remove(ForwardIt, ForwardIt, const T &);
+
+template <typename ForwardIt, typename UnaryPredicate>
+ForwardIt remove_if(ForwardIt, ForwardIt, UnaryPredicate);
+
+template <typename ForwardIt>
+ForwardIt unique(ForwardIt, ForwardIt);
+
+// the check should be able to match std lib calls even if the functions are
+// declared inside inline namespaces
+inline namespace v1 {
+
+template <typename T>
+T *launder(T *);
+
+} // namespace v1
+} // namespace std
+
+struct Foo {
+  void f();
+};
+
+int increment(int i) {
+  return i + 1;
+}
+
+void useFuture(const std::future &fut);
+
+void warning() {
+  std::async(increment, 42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::async(std::launch::async, increment, 42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  Foo F;
+  std::launder(&F);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::remove_if(nullptr, nullptr, nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::unique(nullptr, nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  // test discarding return values inside different kinds of statements
+
+  auto Lambda = [] { std::remove(nullptr, nullptr, 1); };
+  // CHECK-MESSAGES: [[@LINE-1]]:22: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  if (true)
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  else if (true)
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  else
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  while (true)
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  do
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  while (true);
+
+  for (;;)
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  for (std::remove(nullptr, nullptr, 1);;)
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the value returned by this function should be used [bugprone-unused-return-value]
+    ;
+
+  for (;; std::remove(nullptr, nullptr, 1))
+    // CHECK-MESSAGES: [[@LINE-1]]:11: warning: the value returned by this function should be used [bugprone-unused-return-value]
+    ;
+
+  for (auto C : "foo")
+    std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  switch (1) {
+  case 1:
+    std::remove(nullptr, nullptr, 1);
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+    break;
+  default:
+    std::remove(nullptr, nullptr, 1);
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+    break;
+  }
+
+  try {
+    std::remove(nullptr, nullptr, 1);
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  } catch (...) {
+    std::remove(nullptr, nullptr, 1);
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  }
+}
+
+void noWarning() {
+  auto AsyncRetval1 = std::async(increment, 42);
+  auto AsyncRetval2 = std::async(std::launch::async, increment, 42);
+
+  Foo FNoWarning;
+  auto LaunderRetval = std::launder(&FNoWarning);
+
+  auto RemoveRetval = std::remove(nullptr, nullptr, 1);
+
+  auto RemoveIfRetval = std::remove_if(nullptr, nullptr, nullptr);
+
+  auto UniqueRetval = std::unique(nullptr, nullptr);
+
+  // test using the return value in different kinds of expressions
+  useFuture(std::async(increment, 42));
+  std::launder(&FNoWarning)->f();
+  delete std::launder(&FNoWarning);
+
+  if (std::launder(&FNoWarning))
+    ;
+  for (; std::launder(&FNoWarning);)
+    ;
+  while (std::launder(&FNoWarning))
+    ;
+  do
+    ;
+  while (std::launder(&FNoWarning));
+  switch (std::unique(1, 1))
+    ;
+
+  // cast to void should allow ignoring the return value
+  (void)std::async(increment, 42);
+
+  // test discarding return value of functions that are not configured to be checked
+  increment(1);
+
+  // test that the check is disabled inside GNU statement expressions
+  ({ std::async(increment, 42); });
+  auto StmtExprRetval = ({ std::async(increment, 42); });
+}




More information about the cfe-commits mailing list