[libcxx-commits] [PATCH] D156181: [libc++] Categorize mdspan assertions, and move assertions tests

[Christian Trott via Phabricator via libcxx-commits]

crtrott added inline comments.


================
Comment at: libcxx/include/__mdspan/mdspan.h:191
   _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](_OtherIndexTypes... __indices) const {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(extents(), __indices...),
-                                 "mdspan: operator[] out of bounds access");
+    // Note the standard layouts would also check this, but user provided ones may not, so we
+    // check the precondition here
----------------
var-const wrote:
> Is there any case where if we don't do the check inside `layout_left`/`layout_right`, this assertion in `mdspan` would not catch it?
Only if one would use layout_left/right on for their own data structure. For example mdarray will use reuse the layouts, and a user implemented version of that, or for example a user implemented "matrix_type" may not itself check. Generally I do expect the layouts to be used in such types, since its useful machinery beyond mdspan. The reason not to use mdspan in that case is that users may want value semantics of a matrix type, so they just store say a double[16] and a layout_left::mapping<extents<char, 4, 4> or so. 

So the philosophical question is whether we should make the machinery for implementing such types access safe in hardened mode or not. I do believe there is value to doing so and promising essentially that stuff like the standard layouts are safe in hardened mode to be used on their own. But I am not dead-set on it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156181