Skip to content

Conversation

@H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented Nov 8, 2025

...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.

@H-G-Hristov H-G-Hristov requested a review from a team as a code owner November 8, 2025 05:56
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2025

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

...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.


Full diff: https://github.com/llvm/llvm-project/pull/167097.diff

7 Files Affected:

  • (modified) libcxx/include/__memory/inout_ptr.h (+5-3)
  • (modified) libcxx/include/__memory/out_ptr.h (+5-3)
  • (modified) libcxx/include/fstream (+4-4)
  • (modified) libcxx/include/span (+2-2)
  • (added) libcxx/test/libcxx/containers/views/views.span/nodiscard.verify.cpp (+30)
  • (added) libcxx/test/libcxx/input.output/file.streams/fstreams/nodiscard.verify.cpp (+34)
  • (added) libcxx/test/libcxx/utilities/smartptr/adapt/nodiscard.verify.cpp (+50)
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

@github-actions
Copy link

github-actions bot commented Nov 8, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/nodiscard-native_handle branch from 1042cef to 9720f75 Compare November 8, 2025 06:00

_LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI operator void**() const noexcept
Copy link
Contributor

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]].

Copy link
Contributor Author

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.
@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/nodiscard-native_handle branch from 9720f75 to ea08a3c Compare November 8, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants