[libcxx-commits] [libcxx] [libc++] Mark std::unique_ptr::release() as nodiscard (PR #110847)

[Louis Dionne via libcxx-commits]

https://github.com/ldionne created https://github.com/llvm/llvm-project/pull/110847

It's more often than not a bug to call release() while discarding the value, since the resource would leak. While it's technically possible for the user to purposefully do that, I think [[nodiscard]] in that location will do far more good than harm.

Folks can always explicitly discard the result of the expression or use [[maybe_unused]] if the discarding is intended.

Fixes #30246

>From 6faa4993168dabf80801f1f6846834cd14bd0b37 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 2 Oct 2024 10:19:09 -0400
Subject: [PATCH] [libc++] Mark std::unique_ptr::release() as nodiscard

It's more often than not a bug to call release() while discarding the
value, since the resource would leak. While it's technically possible
for the user to purposefully do that, I think [[nodiscard]] in that
location will do far more good than harm.

Folks can always explicitly discard the result of the expression or
use [[maybe_unused]] if the discarding is intended.

Fixes #30246
---
 libcxx/include/__memory/unique_ptr.h          |  2 +-
 libcxx/include/deque                          |  2 +-
 libcxx/include/locale                         |  2 +-
 .../release.nodiscard.verify.cpp              | 21 +++++++++++++++++++
 .../unique.ptr.modifiers/release.pass.cpp     |  6 +++---
 5 files changed, 27 insertions(+), 6 deletions(-)
 create mode 100644 libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.nodiscard.verify.cpp

diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index 11215dc111e36a..38375518551b4d 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -278,7 +278,7 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
     return __ptr_ != nullptr;
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer release() _NOEXCEPT {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 pointer release() _NOEXCEPT {
     pointer __t = __ptr_;
     __ptr_      = pointer();
     return __t;
diff --git a/libcxx/include/deque b/libcxx/include/deque
index bab0526629f0f8..481837260cbd60 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2187,7 +2187,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity() {
     typedef __allocator_destructor<_Allocator> _Dp;
     unique_ptr<pointer, _Dp> __hold(__alloc_traits::allocate(__a, __block_size), _Dp(__a, __block_size));
     __buf.push_back(__hold.get());
-    __hold.release();
+    (void)__hold.release();
 
     for (__map_pointer __i = __map_.end(); __i != __map_.begin();)
       __buf.push_front(*--__i);
diff --git a/libcxx/include/locale b/libcxx/include/locale
index 573910a85bef54..90265d49ebc328 100644
--- a/libcxx/include/locale
+++ b/libcxx/include/locale
@@ -2459,7 +2459,7 @@ _LIBCPP_HIDE_FROM_ABI void __double_or_nothing(unique_ptr<_Tp, void (*)(void*)>&
   if (__t == 0)
     __throw_bad_alloc();
   if (__owns)
-    __b.release();
+    (void)__b.release();
   __b = unique_ptr<_Tp, void (*)(void*)>(__t, free);
   __new_cap /= sizeof(_Tp);
   __n = __b.get() + __n_off;
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.nodiscard.verify.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.nodiscard.verify.cpp
new file mode 100644
index 00000000000000..93131a09f2b64f
--- /dev/null
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.nodiscard.verify.cpp
@@ -0,0 +1,21 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: stdlib=libc++
+
+// <memory>
+//
+// unique_ptr
+//
+// constexpr pointer release() noexcept; // nodiscard as an extension
+
+#include <memory>
+
+void f(std::unique_ptr<int> p) {
+  p.release(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}
diff --git a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.pass.cpp b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.pass.cpp
index 7aedac99030cba..0bb906a13dd3df 100644
--- a/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.pass.cpp
+++ b/libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/release.pass.cpp
@@ -7,10 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 // <memory>
-
+//
 // unique_ptr
-
-// test release
+//
+// constexpr pointer release() noexcept;
 
 #include <memory>
 #include <cassert>