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( "bugprone-undefined-memory-manipulation"); + CheckFactories.registerCheck( + "bugprone-unused-return-value"); CheckFactories.registerCheck( "bugprone-use-after-move"); CheckFactories.registerCheck( 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/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 + +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 FuncRegex; +}; + +} // 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 "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/Regex.h" +#include "llvm/Support/raw_ostream.h" +#include + +using namespace clang::ast_matchers; +using namespace clang::ast_matchers::internal; + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { + +// Same as matchesName, but instead of fully qualified name, matches on a name +// where inline namespaces have been ignored. +AST_MATCHER_P(NamedDecl, matchesInlinedName, std::string, RegExp) { + assert(!RegExp.empty()); + llvm::SmallString<128> InlinedName("::"); + llvm::raw_svector_ostream OS(InlinedName); + PrintingPolicy Policy = Node.getASTContext().getPrintingPolicy(); + Policy.SuppressUnwrittenScope = true; + Node.printQualifiedName(OS, Policy); + return llvm::Regex(RegExp).match(OS.str()); +} + +} // namespace + +UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + FuncRegex(Options.get("FunctionsRegex", + "^::std::async$|" + "^::std::launder$|" + "^::std::unique_ptr<.*>::release$|" + "^::std::.*::allocate$|" + "^::std::(.*::)*empty$")) {} + +void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "FunctionsRegex", FuncRegex); +} + +void UnusedReturnValueCheck::registerMatchers(MatchFinder *Finder) { + // Detect unused return values by finding CallExprs with CompoundStmt parent, + // ignoring any implicit nodes and parentheses in between. + // Disable the check inside GNU statement expressions by ignoring + // CompoundStmts with StmtExpr parent. + Finder->addMatcher( + compoundStmt( + forEach(expr(ignoringImplicit(ignoringParenImpCasts( + callExpr(callee(functionDecl(matchesInlinedName(FuncRegex)))) + .bind("match"))))), + unless(hasParent(stmtExpr()))), + 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: 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 + `_ check + + Warns on unused function return values. + - New `cppcoreguidelines-owning-memory `_ check This check implements the type-based semantic of ``gsl::owner``, but without Index: docs/clang-tidy/checks/bugprone-unused-return-value.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/bugprone-unused-return-value.rst @@ -0,0 +1,32 @@ +.. 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:: FunctionsRegex + + A regular expression specifying the functions to check. Defaults to + ``^::std::async$|^::std::launder$|^::std::unique_ptr<.*>::release$|^::std::.*::allocate$|^::std::(.*::)*empty$``. + 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::unique_ptr::release()``. Not using the return value can lead to + resource leaks if the same pointer isn't stored anywhere else. Often, + ignoring the ``release()`` return value indicates that the programmer + confused the function with ``reset()``. + - All ``allocate()`` calls in ``std``. For example + ``std::allocator::allocate()`` and + ``std::pmr::memory_resource::allocate()``. Not using the return value is a + resource leak. + - All ``empty()`` calls in ``std``. For example ``std::vector::empty()``, + ``std::filesystem::path::empty()`` and ``std::empty()``. Not using the + return value often indicates that the programmer confused the function with + ``clear()``. 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) Index: test/clang-tidy/bugprone-unused-return-value-custom.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-unused-return-value-custom.cpp @@ -0,0 +1,79 @@ +// RUN: %check_clang_tidy %s bugprone-unused-return-value %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: bugprone-unused-return-value.FunctionsRegex, \ +// RUN: value: "^::fun$|^::ns::Template<.*>::Inner::memFun$|^::ns::Type::staticFun$"}]}' \ +// RUN: -- + +namespace std { + +template +T *launder(T *); + +} // namespace std + +namespace ns { + +template +struct Template { + struct Inner { + bool memFun(); + }; +}; + +using IntType = Template; + +struct Derived : public Template::Inner {}; + +struct Retval { + int *P; + Retval() { P = new int; } + ~Retval() { delete P; } +}; + +struct Type { + Retval memFun(); + static Retval staticFun(); +}; + +} // namespace ns + +int fun(); + +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::Template::Inner ObjA1; + ObjA1.memFun(); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + ns::IntType::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::Template::Inner ObjB1; + auto R2 = ObjB1.memFun(); + + auto R3 = ns::Type::staticFun(); + + // 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: test/clang-tidy/bugprone-unused-return-value.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-unused-return-value.cpp @@ -0,0 +1,212 @@ +// RUN: %check_clang_tidy %s bugprone-unused-return-value %t + +namespace std { + +using size_t = decltype(sizeof(int)); + +using max_align_t = long double; + +struct future {}; + +enum class launch { + async, + deferred +}; + +template +future async(Function &&, Args &&...); + +template +future async(launch, Function &&, Args &&...); + +template +bool empty(const T &); + +// 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 + +template +struct allocator { + using value_type = T; + T *allocate(std::size_t); + T *allocate(std::size_t, const void *); +}; + +template +struct allocator_traits { + using value_type = typename Alloc::value_type; + using pointer = value_type *; + using size_type = size_t; + using const_void_pointer = const void *; + static pointer allocate(Alloc &, size_type); + static pointer allocate(Alloc &, size_type, const_void_pointer); +}; + +template +struct scoped_allocator_adaptor : public OuterAlloc { + using pointer = typename allocator_traits::pointer; + using size_type = typename allocator_traits::size_type; + using const_void_pointer = typename allocator_traits::const_void_pointer; + pointer allocate(size_type); + pointer allocate(size_type, const_void_pointer); +}; + +template +struct default_delete {}; + +template > +struct unique_ptr { + using pointer = T *; + pointer release() noexcept; +}; + +template > +struct vector { + bool empty() const noexcept; +}; + +namespace filesystem { + +struct path { + bool empty() const noexcept; +}; + +} // namespace filesystem + +namespace pmr { + +struct memory_resource { + void *allocate(size_t, size_t = alignof(max_align_t)); +}; + +template +struct polymorphic_allocator { + T *allocate(size_t); +}; + +} // namespace pmr + +} // namespace std + +struct Foo { + void f(); +}; + +int increment(int i) { + return i + 1; +} + +void useFuture(const std::future &fut); + +struct FooAlloc { + using value_type = Foo; +}; + +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::unique_ptr UP; + UP.release(); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::allocator FA; + FA.allocate(1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + FA.allocate(1, nullptr); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::allocator_traits>::allocate(FA, 1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + std::allocator_traits>::allocate(FA, 1, nullptr); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::scoped_allocator_adaptor SAA; + SAA.allocate(1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + SAA.allocate(1, nullptr); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::pmr::memory_resource MR; + MR.allocate(1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::pmr::polymorphic_allocator PA; + PA.allocate(1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::vector FV; + FV.empty(); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::filesystem::path P; + P.empty(); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value] + + std::empty(FV); + // CHECK-MESSAGES: [[@LINE-1]]:3: 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); + + std::unique_ptr UPNoWarning; + auto ReleaseRetval = UPNoWarning.release(); + + std::allocator FANoWarning; + auto AllocRetval1 = FANoWarning.allocate(1, nullptr); + auto AllocRetval2 = FANoWarning.allocate(1); + + auto AllocRetval3 = std::allocator_traits>::allocate(FANoWarning, 1); + auto AllocRetval4 = std::allocator_traits>::allocate(FANoWarning, 1, nullptr); + + std::scoped_allocator_adaptor SAANoWarning; + auto AllocRetval5 = SAANoWarning.allocate(1); + auto AllocRetval6 = SAANoWarning.allocate(1, nullptr); + + std::pmr::memory_resource MRNoWarning; + auto AllocRetval7 = MRNoWarning.allocate(1); + + std::pmr::polymorphic_allocator PANoWarning; + auto AllocRetval8 = PANoWarning.allocate(1); + + std::vector FVNoWarning; + auto VectorEmptyRetval = FVNoWarning.empty(); + + std::filesystem::path PNoWarning; + auto PathEmptyRetval = PNoWarning.empty(); + + auto EmptyRetval = std::empty(FVNoWarning); + + // test using the return value in different kinds of expressions + useFuture(std::async(increment, 42)); + std::launder(&FNoWarning)->f(); + delete std::launder(&FNoWarning); + + // 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); }); +}