[libc++][mdspan] Applied [[nodiscard]]

[H-G-Hristov]

[[nodiscard]] should be applied to functions where discarding the return value is most likely a correctness issue.

• https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

[[nodiscard]] should be applied to functions where discarding the return value is most likely a correctness issue.

• https://libcxx.llvm.org/CodingGuidelines.html#apply-nodiscard-where-relevant

---

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

3 Files Affected:

• (modified) libcxx/include/__mdspan/extents.h (+6-4)
• (modified) libcxx/include/__mdspan/mdspan.h (+25-18)
• (added) libcxx/test/libcxx/containers/views/mdspan/nodiscard.verify.cpp (+64)

diff --git a/libcxx/include/__mdspan/extents.h b/libcxx/include/__mdspan/extents.h
index 26219557dbae9..d16bbd2af44f1 100644
--- a/libcxx/include/__mdspan/extents.h
+++ b/libcxx/include/__mdspan/extents.h
@@ -299,11 +299,13 @@ class extents {
 
 public:
   // [mdspan.extents.obs], observers of multidimensional index space
-  _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank() noexcept { return __rank_; }
-  _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank_dynamic() noexcept { return __rank_dynamic_; }
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank() noexcept { return __rank_; }
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank_dynamic() noexcept { return __rank_dynamic_; }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr index_type extent(rank_type __r) const noexcept { return __vals_.__value(__r); }
-  _LIBCPP_HIDE_FROM_ABI static constexpr size_t static_extent(rank_type __r) noexcept {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr index_type extent(rank_type __r) const noexcept {
+    return __vals_.__value(__r);
+  }
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr size_t static_extent(rank_type __r) noexcept {
     return _Values::__static_value(__r);
   }
 
diff --git a/libcxx/include/__mdspan/mdspan.h b/libcxx/include/__mdspan/mdspan.h
index c0f27678197ce..9f3139a874ff9 100644
--- a/libcxx/include/__mdspan/mdspan.h
+++ b/libcxx/include/__mdspan/mdspan.h
@@ -87,12 +87,14 @@ class mdspan {
   using data_handle_type = typename accessor_type::data_handle_type;
   using reference        = typename accessor_type::reference;
 
-  _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank() noexcept { return extents_type::rank(); }
-  _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank_dynamic() noexcept { return extents_type::rank_dynamic(); }
-  _LIBCPP_HIDE_FROM_ABI static constexpr size_t static_extent(rank_type __r) noexcept {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank() noexcept { return extents_type::rank(); }
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr rank_type rank_dynamic() noexcept {
+    return extents_type::rank_dynamic();
+  }
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr size_t static_extent(rank_type __r) noexcept {
     return extents_type::static_extent(__r);
   }
-  _LIBCPP_HIDE_FROM_ABI constexpr index_type extent(rank_type __r) const noexcept {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr index_type extent(rank_type __r) const noexcept {
     return __map_.extents().extent(__r);
   };
 
@@ -185,7 +187,7 @@ class mdspan {
     requires((is_convertible_v<_OtherIndexTypes, index_type> && ...) &&
              (is_nothrow_constructible_v<index_type, _OtherIndexTypes> && ...) &&
              (sizeof...(_OtherIndexTypes) == rank()))
-  _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](_OtherIndexTypes... __indices) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](_OtherIndexTypes... __indices) const {
     // Note the standard layouts would also check this, but user provided ones may not, so we
     // check the precondition here
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(extents(), __indices...),
@@ -196,7 +198,8 @@ class mdspan {
   template <class _OtherIndexType>
     requires(is_convertible_v<const _OtherIndexType&, index_type> &&
              is_nothrow_constructible_v<index_type, const _OtherIndexType&>)
-  _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](const array< _OtherIndexType, rank()>& __indices) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr reference
+  operator[](const array< _OtherIndexType, rank()>& __indices) const {
     return __acc_.access(__ptr_, [&]<size_t... _Idxs>(index_sequence<_Idxs...>) {
       return __map_(__indices[_Idxs]...);
     }(make_index_sequence<rank()>()));
@@ -205,7 +208,7 @@ class mdspan {
   template <class _OtherIndexType>
     requires(is_convertible_v<const _OtherIndexType&, index_type> &&
              is_nothrow_constructible_v<index_type, const _OtherIndexType&>)
-  _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](span<_OtherIndexType, rank()> __indices) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](span<_OtherIndexType, rank()> __indices) const {
     return __acc_.access(__ptr_, [&]<size_t... _Idxs>(index_sequence<_Idxs...>) {
       return __map_(__indices[_Idxs]...);
     }(make_index_sequence<rank()>()));
@@ -237,24 +240,28 @@ class mdspan {
     swap(__x.__acc_, __y.__acc_);
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr const extents_type& extents() const noexcept { return __map_.extents(); };
-  _LIBCPP_HIDE_FROM_ABI constexpr const data_handle_type& data_handle() const noexcept { return __ptr_; };
-  _LIBCPP_HIDE_FROM_ABI constexpr const mapping_type& mapping() const noexcept { return __map_; };
-  _LIBCPP_HIDE_FROM_ABI constexpr const accessor_type& accessor() const noexcept { return __acc_; };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const extents_type& extents() const noexcept {
+    return __map_.extents();
+  };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const data_handle_type& data_handle() const noexcept { return __ptr_; };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const mapping_type& mapping() const noexcept { return __map_; };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const accessor_type& accessor() const noexcept { return __acc_; };
 
   // per LWG-4021 "mdspan::is_always_meow() should be noexcept"
-  _LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_unique() noexcept { return mapping_type::is_always_unique(); };
-  _LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_exhaustive() noexcept {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_unique() noexcept {
+    return mapping_type::is_always_unique();
+  };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_exhaustive() noexcept {
     return mapping_type::is_always_exhaustive();
   };
-  _LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_strided() noexcept {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_strided() noexcept {
     return mapping_type::is_always_strided();
   };
 
-  _LIBCPP_HIDE_FROM_ABI constexpr bool is_unique() const { return __map_.is_unique(); };
-  _LIBCPP_HIDE_FROM_ABI constexpr bool is_exhaustive() const { return __map_.is_exhaustive(); };
-  _LIBCPP_HIDE_FROM_ABI constexpr bool is_strided() const { return __map_.is_strided(); };
-  _LIBCPP_HIDE_FROM_ABI constexpr index_type stride(rank_type __r) const { return __map_.stride(__r); };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool is_unique() const { return __map_.is_unique(); };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool is_exhaustive() const { return __map_.is_exhaustive(); };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool is_strided() const { return __map_.is_strided(); };
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr index_type stride(rank_type __r) const { return __map_.stride(__r); };
 
 private:
   _LIBCPP_NO_UNIQUE_ADDRESS data_handle_type __ptr_{};
diff --git a/libcxx/test/libcxx/containers/views/mdspan/nodiscard.verify.cpp b/libcxx/test/libcxx/containers/views/mdspan/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..5e9a66582ec5c
--- /dev/null
+++ b/libcxx/test/libcxx/containers/views/mdspan/nodiscard.verify.cpp
@@ -0,0 +1,64 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <span>
+
+// Check that functions are marked [[nodiscard]]
+
+#include <array>
+#include <mdspan>
+#include <span>
+
+#include "test_macros.h"
+
+void test() {
+  // mdspan<>
+
+  std::array<int, 4> data;
+  std::mdspan<int, std::extents<std::size_t, 2, 2>> mdsp{data.data(), 2, 2};
+
+  mdsp[0, 1]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::array arr{0, 1};
+  mdsp[arr]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::span sp{arr};
+  mdsp[sp]; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  mdsp.rank();           // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.rank_dynamic();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.static_extent(0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.extent(0);        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  mdsp.extents();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.data_handle(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.mapping();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.accessor();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  mdsp.is_always_unique(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.is_always_exhaustive(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.is_always_strided(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.is_unique();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.is_exhaustive(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.is_strided();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  mdsp.stride(0);       // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  // Helpers
+  
+  std::extents<int, 1, 2> ex;
+  ex.rank();           // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  ex.rank_dynamic();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  ex.static_extent(0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  ex.extent(0);        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  std::dextents<int, 2> dex;
+  dex.rank();           // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  dex.rank_dynamic();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  dex.static_extent(0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  dex.extent(0);        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}

[github-actions]

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

[H-G-Hristov]

Thank you!