khuttun created this revision.
Herald added subscribers: xazax.hun, mgorny.
Detects calls to std::unique_ptr::release where the return value is unused.
https://reviews.llvm.org/D41056
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UniqueptrReleaseUnusedRetvalCheck.cpp
clang-tidy/misc/UniqueptrReleaseUnusedRetvalCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-uniqueptr-release-unused-retval.rst
test/clang-tidy/misc-uniqueptr-release-unused-retval.cpp
Index: test/clang-tidy/misc-uniqueptr-release-unused-retval.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-uniqueptr-release-unused-retval.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s misc-uniqueptr-release-unused-retval %t
+
+namespace std {
+template <typename T>
+struct unique_ptr {
+ T *operator->();
+ T *release();
+};
+} // namespace std
+
+struct Foo {
+ int release();
+};
+
+template <typename T>
+void callRelease(T &t) { t.release(); }
+// CHECK-MESSAGES: [[@LINE-1]]:26: warning: unused std::unique_ptr::release return value [misc-uniqueptr-release-unused-retval]
+
+using FooPtr = std::unique_ptr<Foo>;
+
+template <typename T>
+struct Derived : public std::unique_ptr<T> {
+};
+
+void deleteThis(Foo *pointer) { delete pointer; }
+
+void Warning() {
+ std::unique_ptr<Foo> p1;
+ p1.release();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unused std::unique_ptr::release return value [misc-uniqueptr-release-unused-retval]
+ { p1.release(); }
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: unused std::unique_ptr::release return value [misc-uniqueptr-release-unused-retval]
+ callRelease(p1);
+ FooPtr fp;
+ fp.release();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unused std::unique_ptr::release return value [misc-uniqueptr-release-unused-retval]
+ Derived<Foo> dp;
+ dp.release();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unused std::unique_ptr::release return value [misc-uniqueptr-release-unused-retval]
+}
+
+void NoWarning() {
+ std::unique_ptr<Foo> p2;
+ auto q = p2.release();
+ delete p2.release();
+ deleteThis(p2.release());
+ p2->release();
+ p2.release()->release();
+}
Index: docs/clang-tidy/checks/misc-uniqueptr-release-unused-retval.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-uniqueptr-release-unused-retval.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-uniqueptr-release-unused-retval
+
+misc-uniqueptr-release-unused-retval
+====================================
+
+Warns if the return value of ``std::unique_ptr::release()`` is not used.
+
+Discarding the return value results in leaking the managed object, if the
+pointer isn't stored anywhere else. This can happen for example when
+``release()`` is incorrectly used instead of ``reset()``:
+
+.. code-block:: c++
+
+ void deleteObject() {
+ MyUniquePtr.release();
+ }
+
+The check will warn about this. The fix is to replace the ``release()`` call
+with ``reset()``.
+
+Discarding the ``release()`` return value doesn't necessary result in a leak if
+the pointer is also stored somewhere else:
+
+.. code-block:: c++
+
+ void f(std::unique_ptr<Foo> p) {
+ // store the raw pointer
+ storePointer(p.get());
+
+ // prevent destroying the Foo object when the unique_ptr is destructed
+ p.release();
+ }
+
+The check warns also here. Although there's no leak here, the code can still be
+improved by using the ``release()`` return value:
+
+.. code-block:: c++
+
+ void f(std::unique_ptr<Foo> p) {
+ storePointer(p.release());
+ }
+
+This eliminates the possibility that code causing ``f()`` to return, thus
+causing ``p``'s destructor to be called and making the stored raw pointer
+dangle, is added between ``storePointer()`` and ``release()`` calls.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -142,6 +142,7 @@
misc-throw-by-value-catch-by-reference
misc-unconventional-assign-operator
misc-undelegated-constructor
+ misc-uniqueptr-release-unused-retval
misc-uniqueptr-reset-release
misc-unused-alias-decls
misc-unused-parameters
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,11 @@
Improvements to clang-tidy
--------------------------
+- New `misc-uniqueptr-release-unused-retval
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-uniqueptr-release-unused-retval.html>`_ check
+
+ Detects calls to std::unique_ptr::release where the return value is unused.
+
- New module `fuchsia` for Fuchsia style checks.
- New module `objc` for Objective-C style checks.
Index: clang-tidy/misc/UniqueptrReleaseUnusedRetvalCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/UniqueptrReleaseUnusedRetvalCheck.h
@@ -0,0 +1,35 @@
+//===--- UniqueptrReleaseUnusedRetvalCheck.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_MISC_UNIQUEPTR_RELEASE_UNUSED_RETVAL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RELEASE_UNUSED_RETVAL_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This check flags calls to std::unique_ptr::release with unused return value.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-uniqueptr-release-unused-retval.html
+class UniqueptrReleaseUnusedRetvalCheck : public ClangTidyCheck {
+public:
+ UniqueptrReleaseUnusedRetvalCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RELEASE_UNUSED_RETVAL_H
Index: clang-tidy/misc/UniqueptrReleaseUnusedRetvalCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/UniqueptrReleaseUnusedRetvalCheck.cpp
@@ -0,0 +1,42 @@
+//===--- UniqueptrReleaseUnusedRetvalCheck.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 "UniqueptrReleaseUnusedRetvalCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void UniqueptrReleaseUnusedRetvalCheck::registerMatchers(MatchFinder *Finder) {
+ // match on release() calls with CompoundStmt parent (= unused)
+ auto UniquePtrType = hasType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom("::std::unique_ptr")))));
+ auto ReleaseMethod = cxxMethodDecl(hasName("release"));
+ auto UnusedRetVal = hasParent(compoundStmt());
+ Finder->addMatcher(
+ cxxMemberCallExpr(on(UniquePtrType), callee(ReleaseMethod), UnusedRetVal)
+ .bind("match"),
+ this);
+}
+
+void UniqueptrReleaseUnusedRetvalCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto Matched = Result.Nodes.getNodeAs<Stmt>("match")) {
+ diag(Matched->getLocStart(),
+ "unused std::unique_ptr::release return value");
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -35,6 +35,7 @@
#include "ThrowByValueCatchByReferenceCheck.h"
#include "UnconventionalAssignOperatorCheck.h"
#include "UndelegatedConstructor.h"
+#include "UniqueptrReleaseUnusedRetvalCheck.h"
#include "UniqueptrResetReleaseCheck.h"
#include "UnusedAliasDeclsCheck.h"
#include "UnusedParametersCheck.h"
@@ -94,6 +95,8 @@
"misc-throw-by-value-catch-by-reference");
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
"misc-undelegated-constructor");
+ CheckFactories.registerCheck<UniqueptrReleaseUnusedRetvalCheck>(
+ "misc-uniqueptr-release-unused-retval");
CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
"misc-uniqueptr-reset-release");
CheckFactories.registerCheck<UnusedAliasDeclsCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -27,6 +27,7 @@
SwappedArgumentsCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
UndelegatedConstructor.cpp
+ UniqueptrReleaseUnusedRetvalCheck.cpp
UniqueptrResetReleaseCheck.cpp
UnusedAliasDeclsCheck.cpp
UnusedParametersCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
