khuttun updated this revision to Diff 128879.
khuttun added a comment.
Fix review comments
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,208 @@
+// 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 <typename Function, typename... Args>
+future async(Function &&, Args &&...);
+
+template <typename Function, typename... Args>
+future async(launch, Function &&, Args &&...);
+
+template <typename T>
+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 <typename T>
+T *launder(T *);
+
+} // namespace v1
+
+template <typename T>
+struct allocator {
+ using value_type = T;
+ T *allocate(std::size_t);
+ T *allocate(std::size_t, const void *);
+};
+
+template <typename Alloc>
+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 <typename OuterAlloc, typename... InnerAlloc>
+struct scoped_allocator_adaptor : public OuterAlloc {
+ using pointer = typename allocator_traits<OuterAlloc>::pointer;
+ using size_type = typename allocator_traits<OuterAlloc>::size_type;
+ using const_void_pointer = typename allocator_traits<OuterAlloc>::const_void_pointer;
+ pointer allocate(size_type);
+ pointer allocate(size_type, const_void_pointer);
+};
+
+template <typename T>
+struct default_delete {};
+
+template <typename T, typename Deleter = default_delete<T>>
+struct unique_ptr {
+ using pointer = T *;
+ pointer release() noexcept;
+};
+
+template <typename T, typename Allocator = allocator<T>>
+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 <typename T>
+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<Foo> UP;
+ UP.release();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::allocator<Foo> 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<std::allocator<Foo>>::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<std::allocator<Foo>>::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<FooAlloc, FooAlloc> 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<Foo> 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<Foo> 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<Foo> UPNoWarning;
+ auto ReleaseRetval = UPNoWarning.release();
+
+ std::allocator<Foo> FANoWarning;
+ auto AllocRetval1 = FANoWarning.allocate(1, nullptr);
+ auto AllocRetval2 = FANoWarning.allocate(1);
+
+ auto AllocRetval3 = std::allocator_traits<std::allocator<Foo>>::allocate(FANoWarning, 1);
+ auto AllocRetval4 = std::allocator_traits<std::allocator<Foo>>::allocate(FANoWarning, 1, nullptr);
+
+ std::scoped_allocator_adaptor<FooAlloc, FooAlloc> 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<Foo> PANoWarning;
+ auto AllocRetval8 = PANoWarning.allocate(1);
+
+ std::vector<Foo> 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);
+}
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 <typename T>
+T *launder(T *);
+
+} // namespace std
+
+namespace ns {
+
+template <typename T>
+struct Template {
+ struct Inner {
+ bool memFun();
+ };
+};
+
+using IntType = Template<int>;
+
+struct Derived : public Template<bool>::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<bool>::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<bool>::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: 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,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/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 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,78 @@
+//===--- 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 <cassert>
+
+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.
+ Finder->addMatcher(
+ compoundStmt(forEach(expr(ignoringImplicit(ignoringParenImpCasts(
+ callExpr(callee(functionDecl(matchesInlinedName(FuncRegex))))
+ .bind("match")))))),
+ 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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
