-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[libc++] Added [[nodiscard]] to fstream.native_handle, span.at and in/out_ptr
#167097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[libc++] Added [[nodiscard]] to fstream.native_handle, span.at and in/out_ptr
#167097
Conversation
|
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) Changes...according to Coding Guidelines: At the time of impelentation the ``nodiscard` policy has not been established yet. I did it as a batch as the changes are trivial and minimal. Full diff: https://github.com/llvm/llvm-project/pull/167097.diff 7 Files Affected:
diff --git a/libcxx/include/__memory/inout_ptr.h b/libcxx/include/__memory/inout_ptr.h
index ef345fe469bca..e9de089b8e74e 100644
--- a/libcxx/include/__memory/inout_ptr.h
+++ b/libcxx/include/__memory/inout_ptr.h
@@ -79,9 +79,11 @@ class inout_ptr_t {
}
}
- _LIBCPP_HIDE_FROM_ABI operator _Pointer*() const noexcept { return std::addressof(const_cast<_Pointer&>(__p_)); }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI operator _Pointer*() const noexcept {
+ return std::addressof(const_cast<_Pointer&>(__p_));
+ }
- _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
requires(!is_same_v<_Pointer, void*>)
{
static_assert(is_pointer_v<_Pointer>, "The conversion to void** requires _Pointer to be a raw pointer.");
@@ -96,7 +98,7 @@ class inout_ptr_t {
};
template <class _Pointer = void, class _Smart, class... _Args>
-_LIBCPP_HIDE_FROM_ABI auto inout_ptr(_Smart& __s, _Args&&... __args) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI auto inout_ptr(_Smart& __s, _Args&&... __args) {
using _Ptr = conditional_t<is_void_v<_Pointer>, __pointer_of_t<_Smart>, _Pointer>;
return std::inout_ptr_t<_Smart, _Ptr, _Args&&...>(__s, std::forward<_Args>(__args)...);
}
diff --git a/libcxx/include/__memory/out_ptr.h b/libcxx/include/__memory/out_ptr.h
index e498e3307b9d6..77a21fb05d7dc 100644
--- a/libcxx/include/__memory/out_ptr.h
+++ b/libcxx/include/__memory/out_ptr.h
@@ -71,9 +71,11 @@ class out_ptr_t {
}
}
- _LIBCPP_HIDE_FROM_ABI operator _Pointer*() const noexcept { return std::addressof(const_cast<_Pointer&>(__p_)); }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI operator _Pointer*() const noexcept {
+ return std::addressof(const_cast<_Pointer&>(__p_));
+ }
- _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
requires(!is_same_v<_Pointer, void*>)
{
static_assert(is_pointer_v<_Pointer>, "The conversion to void** requires _Pointer to be a raw pointer.");
@@ -88,7 +90,7 @@ class out_ptr_t {
};
template <class _Pointer = void, class _Smart, class... _Args>
-_LIBCPP_HIDE_FROM_ABI auto out_ptr(_Smart& __s, _Args&&... __args) {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI auto out_ptr(_Smart& __s, _Args&&... __args) {
using _Ptr = conditional_t<is_void_v<_Pointer>, __pointer_of_t<_Smart>, _Pointer>;
return std::out_ptr_t<_Smart, _Ptr, _Args&&...>(__s, std::forward<_Args>(__args)...);
}
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index 1f88d134fe061..a2eb74b61e76b 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -269,7 +269,7 @@ public:
_LIBCPP_HIDE_FROM_ABI basic_filebuf* __open(int __fd, ios_base::openmode __mode);
basic_filebuf* close();
# if _LIBCPP_STD_VER >= 26
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept {
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept {
_LIBCPP_ASSERT_UNCATEGORIZED(this->is_open(), "File must be opened");
# if defined(_LIBCPP_WIN32API)
return std::__filebuf_windows_native_handle(__file_);
@@ -1165,7 +1165,7 @@ public:
_LIBCPP_HIDE_FROM_ABI basic_filebuf<char_type, traits_type>* rdbuf() const;
# if _LIBCPP_STD_VER >= 26
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept { return rdbuf()->native_handle(); }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept { return rdbuf()->native_handle(); }
# endif
_LIBCPP_HIDE_FROM_ABI bool is_open() const;
void open(const char* __s, ios_base::openmode __mode = ios_base::in);
@@ -1321,7 +1321,7 @@ public:
_LIBCPP_HIDE_FROM_ABI basic_filebuf<char_type, traits_type>* rdbuf() const;
# if _LIBCPP_STD_VER >= 26
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept { return rdbuf()->native_handle(); }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept { return rdbuf()->native_handle(); }
# endif
_LIBCPP_HIDE_FROM_ABI bool is_open() const;
void open(const char* __s, ios_base::openmode __mode = ios_base::out);
@@ -1483,7 +1483,7 @@ public:
_LIBCPP_HIDE_FROM_ABI basic_filebuf<char_type, traits_type>* rdbuf() const;
# if _LIBCPP_STD_VER >= 26
- _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept { return rdbuf()->native_handle(); }
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() const noexcept { return rdbuf()->native_handle(); }
# endif
_LIBCPP_HIDE_FROM_ABI bool is_open() const;
_LIBCPP_HIDE_FROM_ABI void open(const char* __s, ios_base::openmode __mode = ios_base::in | ios_base::out);
diff --git a/libcxx/include/span b/libcxx/include/span
index 3d4f9e4ba7831..68fa3d5987310 100644
--- a/libcxx/include/span
+++ b/libcxx/include/span
@@ -362,7 +362,7 @@ public:
}
# if _LIBCPP_STD_VER >= 26
- _LIBCPP_HIDE_FROM_ABI constexpr reference at(size_type __index) const {
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr reference at(size_type __index) const {
if (__index >= size())
std::__throw_out_of_range("span");
return __data_[__index];
@@ -527,7 +527,7 @@ public:
}
# if _LIBCPP_STD_VER >= 26
- _LIBCPP_HIDE_FROM_ABI constexpr reference at(size_type __index) const {
+ [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr reference at(size_type __index) const {
if (__index >= size())
std::__throw_out_of_range("span");
return __data_[__index];
diff --git a/libcxx/test/libcxx/containers/views/views.span/nodiscard.verify.cpp b/libcxx/test/libcxx/containers/views/views.span/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..6deec90aa0f00
--- /dev/null
+++ b/libcxx/test/libcxx/containers/views/views.span/nodiscard.verify.cpp
@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: std-at-least-c++26
+
+// <span>
+
+// Check that functions are marked [[nodiscard]]
+
+#include <array>
+#include <span>
+#include <vector>
+
+void test() {
+ { // Test with static extent
+ std::array arr{94, 92};
+ std::span sp{arr};
+ sp.at(0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ { // Test with dynamic extent
+ std::vector vec{94, 92};
+ std::span sp{vec};
+ sp.at(0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+}
diff --git a/libcxx/test/libcxx/input.output/file.streams/fstreams/nodiscard.verify.cpp b/libcxx/test/libcxx/input.output/file.streams/fstreams/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..a732c8137d6cc
--- /dev/null
+++ b/libcxx/test/libcxx/input.output/file.streams/fstreams/nodiscard.verify.cpp
@@ -0,0 +1,34 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: std-at-least-c++26
+
+// <fstream>
+
+// Check that functions are marked [[nodiscard]]
+
+#include <fstream>
+
+void test() {
+ {
+ std::filebuf fb;
+ fb.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ {
+ std::fstream fs;
+ fs.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ {
+ std::ifstream fs;
+ fs.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ {
+ std::ofstream fs;
+ fs.native_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+}
\ No newline at end of file
diff --git a/libcxx/test/libcxx/utilities/smartptr/adapt/nodiscard.verify.cpp b/libcxx/test/libcxx/utilities/smartptr/adapt/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..55391f554cc0c
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/smartptr/adapt/nodiscard.verify.cpp
@@ -0,0 +1,50 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: std-at-least-c++23
+
+// <memory>
+
+// Check that functions are marked [[nodiscard]]
+
+#include <memory>
+
+void test() {
+ // clang-format off
+ std::unique_ptr<int> uPtr;
+
+ // [inout.ptr]
+ {
+ { // Test inout_ptr()
+ std::inout_ptr(uPtr); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ { // Test operator Pointer*()
+ const std::inout_ptr_t<std::unique_ptr<int>, int*> iPtr{uPtr};
+ iPtr.operator int**(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ { // Test operator void**()
+ const std::inout_ptr_t<std::unique_ptr<int>, void*> iPtr{uPtr};
+ iPtr.operator void**(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ }
+ // [out.ptr]
+ {
+ { // Test out_ptr()
+ std::out_ptr(uPtr); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ { // Test operator Pointer*()
+ const std::out_ptr_t<std::unique_ptr<int>, int*> oPtr{uPtr};
+ oPtr.operator int**(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ { // Test operator void**()
+ const std::out_ptr_t<std::unique_ptr<int>, void*> oPtr{uPtr};
+ oPtr.operator void**(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ }
+ }
+ // clang-format on
+}
\ No newline at end of file
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
1042cef to
9720f75
Compare
libcxx/include/__memory/out_ptr.h
Outdated
|
|
||
| _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept | ||
| [[nodiscard]] _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think annotating the conversion operators makes a ton of sense. Nobody should ever call ptr.operator void**() and if you write static_cast<void**>(ptr) Clang will diagnose without the [[nodiscard]].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Done!
…and `in/out_ptr` ...according to Coding Guidelines: `[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue. At the time of impelentation the ``nodiscard` policy has not been established yet. I did it as a batch as the changes are trivial and minimal.
9720f75 to
ea08a3c
Compare
...according to Coding Guidelines:
[[nodiscard]]should be applied to functions where discarding the return value is most likely a correctness issue.Changes to:
{filebuf|fstream|ifstream}ofstream}.native_handle()span.at()inout_ptr()out_ptr()At the time of impelentation the
[[nodiscard]]policy has not been established yet. I did it as a batch as the changes are trivial and minimal.