[libcxx-commits] [PATCH] D115838: [libc++] [ranges] Remove the static_assert from ranges::begin and ranges::end.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/access.h:127-131
+    [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp (&__t)[_Np]) const noexcept
+      requires requires { __t + _Np; }
+    {
+      return __t + _Np;
     }
----------------
Quuxplusone wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > I needed to add this `requires` (or, I could add `-> decltype(__t + _Np)` if you'd rather ;)) because otherwise this hard-errors when you try to call it on an array of incomplete type.
> > > I actually suspect that what I've got here might hit the same https://gcc.gnu.org/bugzilla//show_bug.cgi?id=103700 that I worked around in `ranges::begin`. If so, then I'm not sure what we should do.
> > I think I'd rather use `-> decltype(__t + _Np)` and not acknowledge explicitly that we're treating incomplete types any special way here.
> My above comment dates to an earlier version of the diff. IIRC, `requires requires { __t + _Np; }` //did indeed// hit https://gcc.gnu.org/bugzilla//show_bug.cgi?id=103700 , and I believe `-> decltype(__t + _Np)` would also hit it. I'll upload the diff again just to make sure I'm right about that (and also I'll add the new `libcxx/test/libcxx/` test you asked for). If I'm right, then I'll have to continue using `requires (sizeof(*__t) != 0)` here. Because of https://gcc.gnu.org/bugzilla//show_bug.cgi?id=103700 , the constraint will have to be something that doesn't depend on pointer arithmetic. (Test case from 103700 demonstrating GCC 11's problem with pointer arithmetic: https://godbolt.org/z/G7e9hx5cb )
Indeed I was right about that. Using
```
     template <class _Tp, size_t _Np>
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr
     auto operator()(_Tp (&__t)[_Np]) const noexcept
       -> decltype(__t + _Np)
     {
       return __t + _Np;
     }
```
we hit the GCC 11 bug: https://buildkite.com/llvm-project/libcxx-ci/builds/7339#e2494f74-3397-43d8-aa24-326e0275dd39
So I'm going to revert back to
```
     template <class _Tp, size_t _Np>
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr
     auto operator()(_Tp (&__t)[_Np]) const noexcept
       requires (sizeof(*__t) != 0)
     {
       return __t + _Np;
     }
```
, wait to confirm that CI is green again, and land this, unless @ldionne interrupts that plan. :)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115838