[libc++] P3029R1: Better `mdspan`'s CTAD #87873

xiaoyang-sde · 2024-04-06T09:02:29Z

Abstract

This pull request implements P3029R1. The paper discusses the current behavior of mdspan's most common pointer-indices CTAD, where the Extents template parameter is deduced as dextents (dynamic extents), even when passing compile-time constant values. The author believes this behavior is suboptimal, as it doesn't take advantage of the compile-time information. The proposed change suggests deducing static extents if integral_constant-like constants are passed, resulting in more intuitive syntax and less error-prone code.

Reference

llvmbot · 2024-04-06T09:02:58Z

@llvm/pr-subscribers-libcxx

Author: Xiaoyang Liu (xiaoyang-sde)

Changes

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

8 Files Affected:

(modified) libcxx/docs/ReleaseNotes/19.rst (+1)
(modified) libcxx/docs/Status/Cxx2cPapers.csv (+1-1)
(modified) libcxx/include/__fwd/span.h (+23)
(modified) libcxx/include/__mdspan/mdspan.h (+8)
(modified) libcxx/include/mdspan (+5-1)
(modified) libcxx/include/span (+22-1)
(modified) libcxx/test/std/containers/views/mdspan/mdspan/deduction.pass.cpp (+17-1)
(modified) libcxx/test/std/containers/views/views.span/span.cons/deduct.pass.cpp (+11)

diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index abf1570737df9e..4a1fca4ce6db9c 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -46,6 +46,7 @@ Implemented Papers
 - P2872R3 - Remove ``wstring_convert`` From C++26
 - P2302R4 - ``std::ranges::contains``
 - P1659R3 - ``std::ranges::starts_with`` and ``std::ranges::ends_with``
+- P3029R1 - Better ``mdspan``'s CTAD
 
 Improvements and New Features
 -----------------------------
diff --git a/libcxx/docs/Status/Cxx2cPapers.csv b/libcxx/docs/Status/Cxx2cPapers.csv
index af270150fda0c0..f707d996a26ea8 100644
--- a/libcxx/docs/Status/Cxx2cPapers.csv
+++ b/libcxx/docs/Status/Cxx2cPapers.csv
@@ -61,5 +61,5 @@
 "`P1068R11 <https://wg21.link/P1068R11>`__","LWG","Vector API for random number generation","Tokyo March 2024","","",""
 "`P2944R3 <https://wg21.link/P2944R3>`__","LWG","Comparisons for ``reference_wrapper``","Tokyo March 2024","","",""
 "`P2642R6 <https://wg21.link/P2642R6>`__","LWG","Padded ``mdspan`` layouts","Tokyo March 2024","","",""
-"`P3029R1 <https://wg21.link/P3029R1>`__","LWG","Better ``mdspan``'s CTAD","Tokyo March 2024","","",""
+"`P3029R1 <https://wg21.link/P3029R1>`__","LWG","Better ``mdspan``'s CTAD","Tokyo March 2024","|Complete|","19.0",""
 "","","","","","",""
diff --git a/libcxx/include/__fwd/span.h b/libcxx/include/__fwd/span.h
index 8dafa742c19df5..2a438254bc4468 100644
--- a/libcxx/include/__fwd/span.h
+++ b/libcxx/include/__fwd/span.h
@@ -10,7 +10,13 @@
 #ifndef _LIBCPP___FWD_SPAN_H
 #define _LIBCPP___FWD_SPAN_H
 
+#include <__concepts/convertible_to.h>
+#include <__concepts/equality_comparable.h>
 #include <__config>
+#include <__type_traits/integral_constant.h>
+#include <__type_traits/is_integral.h>
+#include <__type_traits/is_same.h>
+#include <__type_traits/remove_const.h>
 #include <cstddef>
 #include <limits>
 
@@ -31,6 +37,23 @@ class span;
 
 #endif
 
+#if _LIBCPP_STD_VER >= 26
+
+template <class _Tp>
+concept __integral_constant_like =
+    is_integral_v<decltype(_Tp::value)> && !is_same_v<bool, remove_const_t<decltype(_Tp::value)>> &&
+    convertible_to<_Tp, decltype(_Tp::value)> && equality_comparable_with<_Tp, decltype(_Tp::value)> &&
+    bool_constant<_Tp() == _Tp::value>::value &&
+    bool_constant<static_cast<decltype(_Tp::value)>(_Tp()) == _Tp::value>::value;
+
+template <class _Tp>
+constexpr size_t __maybe_static_ext = dynamic_extent;
+
+template <__integral_constant_like _Tp>
+constexpr size_t __maybe_static_ext<_Tp> = {_Tp::value};
+
+#endif // _LIBCPP_STD_VER >= 26
+
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
diff --git a/libcxx/include/__mdspan/mdspan.h b/libcxx/include/__mdspan/mdspan.h
index d9a0108d6d3507..ed029ae0a0ea7a 100644
--- a/libcxx/include/__mdspan/mdspan.h
+++ b/libcxx/include/__mdspan/mdspan.h
@@ -17,6 +17,7 @@
 #ifndef _LIBCPP___MDSPAN_MDSPAN_H
 #define _LIBCPP___MDSPAN_MDSPAN_H
 
+#include "__fwd/span.h"
 #include <__assert>
 #include <__config>
 #include <__fwd/mdspan.h>
@@ -266,10 +267,17 @@ class mdspan {
   friend class mdspan;
 };
 
+#  if _LIBCPP_STD_VER < 26
 template <class _ElementType, class... _OtherIndexTypes>
   requires((is_convertible_v<_OtherIndexTypes, size_t> && ...) && (sizeof...(_OtherIndexTypes) > 0))
 explicit mdspan(_ElementType*, _OtherIndexTypes...)
     -> mdspan<_ElementType, dextents<size_t, sizeof...(_OtherIndexTypes)>>;
+#  else
+template <class _ElementType, class... _OtherIndexTypes>
+  requires((is_convertible_v<_OtherIndexTypes, size_t> && ...) && (sizeof...(_OtherIndexTypes) > 0))
+explicit mdspan(_ElementType*, _OtherIndexTypes...)
+    -> mdspan<_ElementType, extents<size_t, __maybe_static_ext<_OtherIndexTypes>...>>;
+#  endif
 
 template <class _Pointer>
   requires(is_pointer_v<remove_reference_t<_Pointer>>)
diff --git a/libcxx/include/mdspan b/libcxx/include/mdspan
index c13d9eef001ac9..76f34b67e41004 100644
--- a/libcxx/include/mdspan
+++ b/libcxx/include/mdspan
@@ -370,7 +370,11 @@ namespace std {
   template<class ElementType, class... Integrals>
     requires((is_convertible_v<Integrals, size_t> && ...) && sizeof...(Integrals) > 0)
     explicit mdspan(ElementType*, Integrals...)
-      -> mdspan<ElementType, dextents<size_t, sizeof...(Integrals)>>;
+      -> mdspan<ElementType, dextents<size_t, sizeof...(Integrals)>>;            // until C++26
+  template<class ElementType, class... Integrals>
+    requires((is_convertible_v<Integrals, size_t> && ...) && sizeof...(Integrals) > 0)
+    explicit mdspan(ElementType*, Integrals...)
+      -> mdspan<ElementType, dextents<size_t, maybe-static-ext<Integrals>...>>;  // since C++26
 
   template<class ElementType, class OtherIndexType, size_t N>
     mdspan(ElementType*, span<OtherIndexType, N>)
diff --git a/libcxx/include/span b/libcxx/include/span
index c0fe25ddb4beb9..7c430036478d55 100644
--- a/libcxx/include/span
+++ b/libcxx/include/span
@@ -18,6 +18,20 @@ namespace std {
 // constants
 inline constexpr size_t dynamic_extent = numeric_limits<size_t>::max();
 
+template<class T>
+  concept integral-constant-like =                           // exposition only, since C++26
+    is_integral_v<decltype(T::value)> &&
+    !is_same_v<bool, remove_const_t<decltype(T::value)>> &&
+    convertible_to<T, decltype(T::value)> &&
+    equality_comparable_with<T, decltype(T::value)> &&
+    bool_constant<T() == T::value>::value &&
+    bool_constant<static_cast<decltype(T::value)>(T()) == T::value>::value;
+
+template<class T>
+  constexpr size_t maybe-static-ext = dynamic_extent;        // exposition only, since C++26
+template<integral-constant-like T>
+  constexpr size_t maybe-static-ext<T> = {T::value};
+
 // [views.span], class template span
 template <class ElementType, size_t Extent = dynamic_extent>
     class span;
@@ -110,7 +124,9 @@ private:
 };
 
 template<class It, class EndOrSize>
-    span(It, EndOrSize) -> span<remove_reference_t<iter_reference_t<_It>>>;
+    span(It, EndOrSize) -> span<remove_reference_t<iter_reference_t<_It>>>;                             // until C++26
+template<class It, class EndOrSize>
+    span(It, EndOrSize) -> span<remove_reference_t<iter_reference_t<It>>, maybe-static-ext<EndOrSize>>; // since C++26
 
 template<class T, size_t N>
     span(T (&)[N]) -> span<T, N>;
@@ -563,8 +579,13 @@ _LIBCPP_HIDE_FROM_ABI auto as_writable_bytes(span<_Tp, _Extent> __s) noexcept {
   return __s.__as_writable_bytes();
 }
 
+#if _LIBCPP_STD_VER < 26
 template <contiguous_iterator _It, class _EndOrSize>
 span(_It, _EndOrSize) -> span<remove_reference_t<iter_reference_t<_It>>>;
+#else
+template <contiguous_iterator _It, class _EndOrSize>
+span(_It, _EndOrSize) -> span<remove_reference_t<iter_reference_t<_It>>, __maybe_static_ext<_EndOrSize>>;
+#endif
 
 template <class _Tp, size_t _Sz>
 span(_Tp (&)[_Sz]) -> span<_Tp, _Sz>;
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/deduction.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/deduction.pass.cpp
index 557829dcbad9be..8d3a86910e33d7 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/deduction.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/deduction.pass.cpp
@@ -22,7 +22,11 @@
 //  template<class ElementType, class... Integrals>
 //    requires((is_convertible_v<Integrals, size_t> && ...) && sizeof...(Integrals) > 0)
 //    explicit mdspan(ElementType*, Integrals...)
-//      -> mdspan<ElementType, dextents<size_t, sizeof...(Integrals)>>;
+//      -> mdspan<ElementType, dextents<size_t, sizeof...(Integrals)>>;            // until C++26
+//  template<class ElementType, class... Integrals>
+//    requires((is_convertible_v<Integrals, size_t> && ...) && sizeof...(Integrals) > 0)
+//    explicit mdspan(ElementType*, Integrals...)
+//      -> mdspan<ElementType, dextents<size_t, maybe-static-ext<Integrals>...>>;  // since C++26
 //
 //  template<class ElementType, class OtherIndexType, size_t N>
 //    mdspan(ElementType*, span<OtherIndexType, N>)
@@ -102,6 +106,18 @@ constexpr bool test_no_layout_deduction_guides(const H& handle, const A&) {
   // deduction from pointer and integral like
   ASSERT_SAME_TYPE(decltype(std::mdspan(handle, 5, SizeTIntType(6))), std::mdspan<T, std::dextents<size_t, 2>>);
 
+#if _LIBCPP_STD_VER >= 26
+  // P3029R1: deduction from `integral_constant`
+  ASSERT_SAME_TYPE(
+      decltype(std::mdspan(handle, std::integral_constant<size_t, 5>{})), std::mdspan<T, std::extents<size_t, 5>>);
+  ASSERT_SAME_TYPE(decltype(std::mdspan(handle, std::integral_constant<size_t, 5>{}, std::dynamic_extent)),
+                   std::mdspan<T, std::extents<size_t, 5, std::dynamic_extent>>);
+  ASSERT_SAME_TYPE(
+      decltype(std::mdspan(
+          handle, std::integral_constant<size_t, 5>{}, std::dynamic_extent, std::integral_constant<size_t, 7>{})),
+      std::mdspan<T, std::extents<size_t, 5, std::dynamic_extent, 7>>);
+#endif
+
   std::array<char, 3> exts;
   // deduction from pointer and array
   ASSERT_SAME_TYPE(decltype(std::mdspan(handle, exts)), std::mdspan<T, std::dextents<size_t, 3>>);
diff --git a/libcxx/test/std/containers/views/views.span/span.cons/deduct.pass.cpp b/libcxx/test/std/containers/views/views.span/span.cons/deduct.pass.cpp
index 398862dccdb6a1..92bb7c9caea836 100644
--- a/libcxx/test/std/containers/views/views.span/span.cons/deduct.pass.cpp
+++ b/libcxx/test/std/containers/views/views.span/span.cons/deduct.pass.cpp
@@ -31,6 +31,7 @@
 #include <iterator>
 #include <memory>
 #include <string>
+#include <type_traits>
 
 #include "test_macros.h"
 
@@ -48,6 +49,16 @@ void test_iterator_sentinel() {
   assert(s.size() == std::size(arr));
   assert(s.data() == std::data(arr));
   }
+
+#if _LIBCPP_STD_VER >= 26
+  // P3029R1: deduction from `integral_constant`
+  {
+    std::span s{std::begin(arr), std::integral_constant<size_t, 3>{}};
+    ASSERT_SAME_TYPE(decltype(s), std::span<int, 3>);
+    assert(s.size() == std::size(arr));
+    assert(s.data() == std::data(arr));
+  }
+#endif
 }
 
 void test_c_array() {

github-actions · 2024-04-06T09:05:26Z

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

libcxx/include/__fwd/span.h

mordante

Thanks for your contribution. A few minor comments.

libcxx/include/span

libcxx/include/mdspan

mordante

Thanks! LGTM!

xiaoyang-sde · 2024-04-12T16:16:17Z

Thanks! LGTM!

Thanks for the code review! Would you mind merging this pull request if it looks good? I don't have the permission to do so.

mordante · 2024-04-12T17:25:46Z

Thanks! LGTM!

Thanks for the code review! Would you mind merging this pull request if it looks good? I don't have the permission to do so.

Thanks for the ping. I just landed the patch.

## Abstract This pull request implements [P3029R1](https://wg21.link/P3029R1). The paper discusses the current behavior of `mdspan`'s most common pointer-indices CTAD, where the `Extents` template parameter is deduced as `dextents` (dynamic extents), even when passing compile-time constant values. The author believes this behavior is suboptimal, as it doesn't take advantage of the compile-time information. The proposed change suggests deducing static extents if `integral_constant`-like constants are passed, resulting in more intuitive syntax and less error-prone code. ## Reference - [P3029R1](https://wg21.link/P3029R1) - [Draft C++ Standard: [span.syn]](https://eel.is/c++draft/span.syn) - [Draft C++ Standard: [mdspan.syn]](https://eel.is/c++draft/mdspan.syn)

hewillk · 2024-04-17T02:22:10Z

You seem to be missing the CTAD for std::extents, which is also in the paper's wording.

xiaoyang-sde · 2024-04-17T03:43:47Z

You seem to be missing the CTAD for std::extents, which is also in the paper's wording.

Thanks for pointing this out! I'm attempting to fix this in #89015.

This patch implements an improvement introduced in P3029R1 that was missed in #87873. It adds a deduction of static extents if integral_constant-like constants are passed to `std::extents`.

This patch implements an improvement introduced in P3029R1 that was missed in llvm#87873. It adds a deduction of static extents if integral_constant-like constants are passed to `std::extents`.

Xiaoyang Liu added 3 commits April 6, 2024 01:58

[libc++] P3029R1: Better mdspan's CTAD

386a5dd

[libc++] P3029R1: Better mdspan's CTAD

f1dc579

[libc++] P3029R1: Better mdspan's CTAD

730a76d

xiaoyang-sde requested a review from a team as a code owner April 6, 2024 09:02

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 6, 2024

xiaoyang-sde changed the title ~~[libc++] P3029R1: Better mdspan's CTAD~~ [libc++] P3029R1: Better mdspan's CTAD Apr 6, 2024

[libc++] P3029R1: Better mdspan's CTAD

6ac8a6c

xiaoyang-sde commented Apr 6, 2024

View reviewed changes

libcxx/include/__fwd/span.h Outdated Show resolved Hide resolved

Xiaoyang Liu added 3 commits April 6, 2024 03:19

[libc++] P3029R1: Better mdspan's CTAD

aa40494

[libc++] P3029R1: Better mdspan's CTAD

a7ff0a0

[libc++] P3029R1: Better mdspan's CTAD

0bc1f4e

mordante reviewed Apr 7, 2024

View reviewed changes

libcxx/include/span Outdated Show resolved Hide resolved

libcxx/include/span Outdated Show resolved Hide resolved

libcxx/include/span Outdated Show resolved Hide resolved

libcxx/include/mdspan Outdated Show resolved Hide resolved

Xiaoyang Liu added 3 commits April 7, 2024 15:21

[libc++] P3029R1: Better mdspan's CTAD

cf2f118

[libc++] P3029R1: Better mdspan's CTAD

a8ee993

[libc++] P3029R1: Better mdspan's CTAD

2f1a094

xiaoyang-sde requested a review from mordante April 8, 2024 03:39

mordante approved these changes Apr 8, 2024

View reviewed changes

mordante self-assigned this Apr 8, 2024

mordante merged commit 7cafe04 into llvm:main Apr 12, 2024

xiaoyang-sde deleted the mdspan_ctad branch April 17, 2024 02:47

xiaoyang-sde mentioned this pull request Apr 17, 2024

[libc++] P3029R1: Better mdspan's CTAD - std::extents #89015

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] P3029R1: Better `mdspan`'s CTAD #87873

[libc++] P3029R1: Better `mdspan`'s CTAD #87873

Uh oh!

xiaoyang-sde commented Apr 6, 2024 •

edited

Loading

Uh oh!

llvmbot commented Apr 6, 2024

Uh oh!

github-actions bot commented Apr 6, 2024 •

edited

Loading

Uh oh!

Uh oh!

mordante left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mordante left a comment

Uh oh!

xiaoyang-sde commented Apr 12, 2024 •

edited

Loading

Uh oh!

mordante commented Apr 12, 2024

Uh oh!

hewillk commented Apr 17, 2024

Uh oh!

xiaoyang-sde commented Apr 17, 2024

Uh oh!

Uh oh!

[libc++] P3029R1: Better mdspan's CTAD #87873

[libc++] P3029R1: Better mdspan's CTAD #87873

Uh oh!

Conversation

xiaoyang-sde commented Apr 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Abstract

Reference

Uh oh!

llvmbot commented Apr 6, 2024

Uh oh!

github-actions bot commented Apr 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

xiaoyang-sde commented Apr 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mordante commented Apr 12, 2024

Uh oh!

hewillk commented Apr 17, 2024

Uh oh!

xiaoyang-sde commented Apr 17, 2024

Uh oh!

Uh oh!

[libc++] P3029R1: Better `mdspan`'s CTAD #87873

[libc++] P3029R1: Better `mdspan`'s CTAD #87873

xiaoyang-sde commented Apr 6, 2024 •

edited

Loading

github-actions bot commented Apr 6, 2024 •

edited

Loading

xiaoyang-sde commented Apr 12, 2024 •

edited

Loading