[clang-tools-extra] r330772 - [clang-tidy] Improve bugprone-unused-return-value check
Jonathan Coe via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 24 14:25:16 PDT 2018
Author: jbcoe
Date: Tue Apr 24 14:25:16 2018
New Revision: 330772
URL: http://llvm.org/viewvc/llvm-project?rev=330772&view=rev
Log:
[clang-tidy] Improve bugprone-unused-return-value check
Summary:
Add support for checking class template member functions.
Also add the following functions to be checked by default:
- std::unique_ptr::release
- std::basic_string::empty
- std::vector::empty
Reviewers: alexfh, hokein, aaron.ballman, ilya-biryukov
Reviewed By: aaron.ballman
Subscribers: jbcoe, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D45891
Patch by khuttun (Kalle Huttunen)
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
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/UnusedReturnValueCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp?rev=330772&r1=330771&r2=330772&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp Tue Apr 24 14:25:16 2018
@@ -19,14 +19,32 @@ namespace clang {
namespace tidy {
namespace bugprone {
+namespace {
+
+// Matches functions that are instantiated from a class template member function
+// matching InnerMatcher. Functions not instantiated from a class template
+// member function are matched directly with InnerMatcher.
+AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, Matcher<FunctionDecl>,
+ InnerMatcher) {
+ FunctionDecl *InstantiatedFrom = Node.getInstantiatedFromMemberFunction();
+ return InnerMatcher.matches(InstantiatedFrom ? *InstantiatedFrom : Node,
+ Finder, Builder);
+}
+
+} // namespace
+
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")) {}
+ CheckedFunctions(Options.get("CheckedFunctions",
+ "::std::async;"
+ "::std::launder;"
+ "::std::remove;"
+ "::std::remove_if;"
+ "::std::unique;"
+ "::std::unique_ptr::release;"
+ "::std::basic_string::empty;"
+ "::std::vector::empty")) {}
void UnusedReturnValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CheckedFunctions", CheckedFunctions);
@@ -35,11 +53,11 @@ void UnusedReturnValueCheck::storeOption
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())))))
+ callExpr(callee(functionDecl(
+ // Don't match void overloads of checked functions.
+ unless(returns(voidType())),
+ isInstantiatedFrom(hasAnyName(
+ std::vector<StringRef>(FunVec.begin(), FunVec.end()))))))
.bind("match"))));
auto UnusedInCompoundStmt =
Modified: 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=330772&r1=330771&r2=330772&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst Tue Apr 24 14:25:16 2018
@@ -11,7 +11,7 @@ Options
.. option:: CheckedFunctions
Semicolon-separated list of functions to check. Defaults to
- ``::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique``.
+ ``::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique;::std::unique_ptr::release;::std::basic_string::empty;::std::vector::empty``.
This means that the calls to following functions are checked by default:
- ``std::async()``. Not using the return value makes the call synchronous.
@@ -22,3 +22,10 @@ Options
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.
+ - ``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()``.
+ - ``std::basic_string::empty()`` and ``std::vector::empty()``. Not using the
+ return value often indicates that the programmer confused the function with
+ ``clear()``.
Modified: 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=330772&r1=330771&r2=330772&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp Tue Apr 24 14:25:16 2018
@@ -1,7 +1,7 @@
// 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: value: "::fun;::ns::Outer::Inner::memFun;::ns::Type::staticFun;::ns::ClassTemplate::memFun;::ns::ClassTemplate::staticFun"}]}' \
// RUN: --
namespace std {
@@ -34,6 +34,12 @@ struct Type {
static Retval staticFun();
};
+template <typename T>
+struct ClassTemplate {
+ Retval memFun();
+ static Retval staticFun();
+};
+
} // namespace ns
int fun();
@@ -60,6 +66,13 @@ void warning() {
ns::Type::staticFun();
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ ns::ClassTemplate<int> ObjA4;
+ ObjA4.memFun();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ ns::ClassTemplate<int>::staticFun();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
}
void noWarning() {
@@ -70,6 +83,11 @@ void noWarning() {
auto R3 = ns::Type::staticFun();
+ ns::ClassTemplate<int> ObjB2;
+ auto R4 = ObjB2.memFun();
+
+ auto R5 = ns::ClassTemplate<int>::staticFun();
+
// test calling a void overload of a checked function
fun(5);
@@ -77,6 +95,6 @@ void noWarning() {
int I = 1;
std::launder(&I);
- ns::Type ObjB2;
- ObjB2.memFun();
+ ns::Type ObjB3;
+ ObjB3.memFun();
}
Modified: 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=330772&r1=330771&r2=330772&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp Tue Apr 24 14:25:16 2018
@@ -24,6 +24,34 @@ ForwardIt remove_if(ForwardIt, ForwardIt
template <typename ForwardIt>
ForwardIt unique(ForwardIt, ForwardIt);
+template <typename T>
+struct default_delete;
+
+template <typename T, typename Deleter = std::default_delete<T>>
+struct unique_ptr {
+ T *release() noexcept;
+};
+
+template <typename T>
+struct char_traits;
+
+template <typename T>
+struct allocator;
+
+template <typename CharT,
+ typename Traits = char_traits<CharT>,
+ typename Allocator = allocator<CharT>>
+struct basic_string {
+ bool empty() const;
+};
+
+typedef basic_string<char> string;
+
+template <typename T, typename Allocator = std::allocator<T>>
+struct vector {
+ bool empty() const noexcept;
+};
+
// the check should be able to match std lib calls even if the functions are
// declared inside inline namespaces
inline namespace v1 {
@@ -64,6 +92,18 @@ void warning() {
std::unique(nullptr, nullptr);
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+ std::unique_ptr<Foo> UPtr;
+ UPtr.release();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::string Str;
+ Str.empty();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::vector<Foo> Vec;
+ Vec.empty();
+ // 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); };
@@ -137,6 +177,15 @@ void noWarning() {
auto UniqueRetval = std::unique(nullptr, nullptr);
+ std::unique_ptr<Foo> UPtrNoWarning;
+ auto ReleaseRetval = UPtrNoWarning.release();
+
+ std::string StrNoWarning;
+ auto StrEmptyRetval = StrNoWarning.empty();
+
+ std::vector<Foo> VecNoWarning;
+ auto VecEmptyRetval = VecNoWarning.empty();
+
// test using the return value in different kinds of expressions
useFuture(std::async(increment, 42));
std::launder(&FNoWarning)->f();
More information about the cfe-commits
mailing list