This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE330772: [clang-tidy] Improve bugprone-unused-return-value
check (authored by jbcoe, committed by ).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45891
Files:
clang-tidy/bugprone/UnusedReturnValueCheck.cpp
docs/clang-tidy/checks/bugprone-unused-return-value.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
===================================================================
--- test/clang-tidy/bugprone-unused-return-value.cpp
+++ test/clang-tidy/bugprone-unused-return-value.cpp
@@ -24,6 +24,34 @@
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 @@
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 @@
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();
Index: test/clang-tidy/bugprone-unused-return-value-custom.cpp
===================================================================
--- test/clang-tidy/bugprone-unused-return-value-custom.cpp
+++ test/clang-tidy/bugprone-unused-return-value-custom.cpp
@@ -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 @@
static Retval staticFun();
};
+template <typename T>
+struct ClassTemplate {
+ Retval memFun();
+ static Retval staticFun();
+};
+
} // namespace ns
int fun();
@@ -60,6 +66,13 @@
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,13 +83,18 @@
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);
// test discarding return value of functions that are not configured to be checked
int I = 1;
std::launder(&I);
- ns::Type ObjB2;
- ObjB2.memFun();
+ ns::Type ObjB3;
+ ObjB3.memFun();
}
Index: clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===================================================================
--- clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -19,27 +19,45 @@
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);
}
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 =
Index: docs/clang-tidy/checks/bugprone-unused-return-value.rst
===================================================================
--- docs/clang-tidy/checks/bugprone-unused-return-value.rst
+++ docs/clang-tidy/checks/bugprone-unused-return-value.rst
@@ -11,7 +11,7 @@
.. 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 @@
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()``.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
