Index: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -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 @@ "bugprone-undelegated-constructor"); CheckFactories.registerCheck( "bugprone-unused-raii"); + CheckFactories.registerCheck( + "bugprone-unused-return-value"); CheckFactories.registerCheck( "bugprone-use-after-move"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt @@ -35,6 +35,7 @@ UndefinedMemoryManipulationCheck.cpp UndelegatedConstructorCheck.cpp UnusedRaiiCheck.cpp + UnusedReturnValueCheck.cpp UseAfterMoveCheck.cpp VirtualNearMissCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h +++ clang-tools-extra/trunk/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 + +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-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp +++ clang-tools-extra/trunk/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( + 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("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-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -67,6 +67,11 @@ Diagnoses when a temporary object that appears to be an exception is constructed but not thrown. +- New `bugprone-unused-return-value + `_ check + + Warns on unused function return values. + - New `cppcoreguidelines-avoid-goto `_ check Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -51,6 +51,7 @@ 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) Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp +++ clang-tools-extra/trunk/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 +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: clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp +++ clang-tools-extra/trunk/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 +future async(Function &&, Args &&...); + +template +future async(launch, Function &&, Args &&...); + +template +ForwardIt remove(ForwardIt, ForwardIt, const T &); + +template +ForwardIt remove_if(ForwardIt, ForwardIt, UnaryPredicate); + +template +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 +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); }); +}