[libcxx-commits] [libcxx] [libc++][mdspan] Add missing `std::move` in `std::extents` (PR #196574)

[A. Jiang via libcxx-commits]

================
@@ -322,7 +322,7 @@ class extents {
              (is_nothrow_constructible_v<index_type, _OtherIndexTypes> && ...) &&
              (sizeof...(_OtherIndexTypes) == __rank_ || sizeof...(_OtherIndexTypes) == __rank_dynamic_))
   _LIBCPP_HIDE_FROM_ABI constexpr explicit extents(_OtherIndexTypes... __dynvals) noexcept
-      : __vals_(static_cast<index_type>(__dynvals)...) {
+      : __vals_(static_cast<index_type>(std::move(__dynvals))...) {
----------------
frederick-vs-ja wrote:

I think we can introduce the following helper function:

```c++
// in namespace __mdspan_detail, after __are_representable_as

template <integral _To, class _From>
_LIBCPP_HIDE_FROM_ABI constexpr _To __representability_checked_cast(_From __value) {
  if constexpr (integral<_From>) {
    using _To_u   = make_unsigned_t<_To>;
    using _From_u = make_unsigned_t<_From>;
    if constexpr (is_signed_v<_From>) {
      _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
          __value >= 0, "extents ctor: arguments must be representable as index_type and nonnegative");
    }
    if constexpr (static_cast<_To_u>(numeric_limits<_To>::max()) < static_cast<_From_u>(numeric_limits<_From>::max())) {
      _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
          static_cast<_To_u>(numeric_limits<_To>::max()) >= static_cast<_From_u>(__value),
          "extents ctor: arguments must be representable as index_type and nonnegative");
    }
    return static_cast<_To>(__value);
  } else {
    auto __converted_val = static_cast<_To>(std::move(__value));
    if constexpr (is_signed_v<_To>) {
      _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
          __converted_val >= 0, "extents ctor: arguments must be representable as index_type and nonnegative");
    }
    return __converted_val;
  }
}
```

And then refactor this constructor as:
```c++
  // Construction from just dynamic or all values.
  // Precondition check is deferred to __maybe_static_array constructor
  template <class... _OtherIndexTypes>
    requires((is_convertible_v<_OtherIndexTypes, index_type> && ...) &&
             (is_nothrow_constructible_v<index_type, _OtherIndexTypes> && ...) &&
             (sizeof...(_OtherIndexTypes) == __rank_ || sizeof...(_OtherIndexTypes) == __rank_dynamic_))
  _LIBCPP_HIDE_FROM_ABI constexpr explicit extents(_OtherIndexTypes... __dynvals) noexcept
      // Not catching this could lead to out of bounds errors later
      // e.g. mdspan m(ptr, dextents<char, 1>(200u)); leads to an extent of -56 on m
      : __vals_(__mdspan_detail::__representability_checked_cast<index_type>(std::move(__dynvals))...) {}
```

Such strategy should be able to avoid double move while keeping checks.

https://github.com/llvm/llvm-project/pull/196574