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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne 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:
> 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. :)
> 
At least, please add a comment like `// make sure _Tp is complete`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115838