[libcxx-commits] [PATCH] D127418: [libc++] Use bounded iterators in std::span when the debug mode is enabled

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/span:214
+#ifdef _LIBCPP_ENABLE_DEBUG_MODE
+    using iterator               = __bounded_iter<pointer>;
 #else
----------------
What do you think about introducing a `constexpr bool _InDebugMode` and make this `using iterator = _If<_InDebugMode, __bounded_iter<pointer>, __wrap_iter<pointer>>`? With that it would also be possible to use `if constexpr (_InDebugMode)` and `void func() requires _InDebugMode`. This only works for C++11, C++17 and C++20 code respectively, but is a lot nicer to read IMO.


================
Comment at: libcxx/include/span:386-390
+#ifdef _LIBCPP_ENABLE_DEBUG_MODE
+        return iterator(data(), data(), data() + size());
 #else
         return iterator(this, data());
 #endif
----------------
For example this could change to
```
        if constexpr (_InDebugMode) {
            return iterator(data(), data(), data() + size());
        } else {
            return iterator(this, data());
        }
```


================
Comment at: libcxx/test/libcxx/containers/views/views.span/span.elem/assert.back.pass.cpp:28
+        int array[] = {0, 1, 2};
+        std::span<int> const s(&array[0], 0);
+        TEST_LIBCPP_ASSERT_FAILURE(s.back(), "span<T>::back() on empty span");
----------------
Maybe `array.data()` or `std::data(array)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127418