[libcxx-commits] [PATCH] D154367: [libc++] mdspan - implement mdspan class

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__mdspan/mdspan.h:61
+template <class _LayoutPolicy>
+_LIBCPP_HIDE_FROM_ABI constexpr bool is_possibly_mapping() {
+  return false;
----------------
_Uglification!


================
Comment at: libcxx/include/__mdspan/mdspan.h:115
+  using element_type     = _ElementType;
+  using value_type       = std::remove_cv_t<element_type>;
+  using index_type       = typename extents_type::index_type;
----------------



================
Comment at: libcxx/include/__mdspan/mdspan.h:130-131
+private:
+  // Can't use defaulted parameter in the __deduction_workaround template because of a bug in MSVC warning C4348.
+  using __impl = __deduction_workaround<std::make_index_sequence<extents_type::rank()>>;
+
----------------
libc++ doesn't work with MSVC anyways, so this workaround can be removed.


================
Comment at: libcxx/include/__mdspan/mdspan.h:256
+private:
+  [[no_unique_address]] data_handle_type __ptr_{};
+  [[no_unique_address]] mapping_type __map_{};
----------------
These should be `_LIBCPP_NO_UNIQUE_ADDRESS`.


================
Comment at: libcxx/include/__mdspan/mdspan.h:274
+template < class CArray>
+  requires(std::is_array_v<CArray> && (rank_v<CArray> == 1))
+mdspan(CArray&) -> mdspan<std::remove_all_extents_t<CArray>, extents<size_t, ::std::extent_v<CArray, 0>>>;
----------------



================
Comment at: libcxx/include/__mdspan/mdspan.h:275
+  requires(std::is_array_v<CArray> && (rank_v<CArray> == 1))
+mdspan(CArray&) -> mdspan<std::remove_all_extents_t<CArray>, extents<size_t, ::std::extent_v<CArray, 0>>>;
+
----------------



================
Comment at: libcxx/include/__mdspan/mdspan.h:278
+template <class _ElementType, class _OtherIndexType, size_t _Size>
+mdspan(_ElementType*, const ::std::array<_OtherIndexType, _Size>&) -> mdspan<_ElementType, dextents<size_t, _Size>>;
+
----------------



================
Comment at: libcxx/include/__mdspan/mdspan.h:281
+template <class _ElementType, class _OtherIndexType, size_t _Size>
+mdspan(_ElementType*, ::std::span<_OtherIndexType, _Size>) -> mdspan<_ElementType, dextents<size_t, _Size>>;
+
----------------



================
Comment at: libcxx/test/std/containers/views/mdspan/mdspan/CustomTestAccessors.h:1
+#include <mdspan>
+#include <type_traits>
----------------
Missing license header and include guard


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154367/new/

https://reviews.llvm.org/D154367