khuttun updated this revision to Diff 134802.
khuttun added a comment.
I changed the checker to use hasAnyName. Checking for `unique_ptr::release()`
and all the `empty()` functions is removed.
The checker doesn't report any warnings from LLVM + clang codebases now.
https://reviews.llvm.org/D41655
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/UnusedReturnValueCheck.cpp
clang-tidy/bugprone/UnusedReturnValueCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-unused-return-value.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-unused-return-value-custom.cpp
test/clang-tidy/bugprone-unused-return-value.cpp
Index: test/clang-tidy/bugprone-unused-return-value.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-unused-return-value.cpp
@@ -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); });
+}
Index: test/clang-tidy/bugprone-unused-return-value-custom.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-unused-return-value-custom.cpp
@@ -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();
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -32,6 +32,7 @@
bugprone-string-constructor
bugprone-suspicious-memset-usage
bugprone-undefined-memory-manipulation
+ bugprone-unused-return-value
bugprone-use-after-move
bugprone-virtual-near-miss
cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
Index: docs/clang-tidy/checks/bugprone-unused-return-value.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-unused-return-value.rst
@@ -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.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -125,6 +125,11 @@
a memory allocation function (``malloc()``, ``calloc()``, ``realloc()``,
``alloca()``) or the ``new[]`` operator in `C++`.
+- 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-owning-memory <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html>`_ check
This check implements the type-based semantic of ``gsl::owner<T*>``, but without
Index: clang-tidy/bugprone/UnusedReturnValueCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/UnusedReturnValueCheck.h
@@ -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
Index: clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -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
Index: clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tidy/bugprone/CMakeLists.txt
+++ clang-tidy/bugprone/CMakeLists.txt
@@ -17,6 +17,7 @@
StringConstructorCheck.cpp
SuspiciousMemsetUsageCheck.cpp
UndefinedMemoryManipulationCheck.cpp
+ UnusedReturnValueCheck.cpp
UseAfterMoveCheck.cpp
VirtualNearMissCheck.cpp
Index: clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -25,6 +25,7 @@
#include "StringConstructorCheck.h"
#include "SuspiciousMemsetUsageCheck.h"
#include "UndefinedMemoryManipulationCheck.h"
+#include "UnusedReturnValueCheck.h"
#include "UseAfterMoveCheck.h"
#include "VirtualNearMissCheck.h"
@@ -65,6 +66,8 @@
"bugprone-suspicious-memset-usage");
CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
"bugprone-undefined-memory-manipulation");
+ CheckFactories.registerCheck<UnusedReturnValueCheck>(
+ "bugprone-unused-return-value");
CheckFactories.registerCheck<UseAfterMoveCheck>(
"bugprone-use-after-move");
CheckFactories.registerCheck<VirtualNearMissCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
