[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:63-64
   struct __fn {
+    // The Standard says "t + 0", but that triggers GCC bug 103700,
+    // so decay to a pointer via plain old "__t" instead.
     template <class _Tp>
----------------
This may be a bad comment. At least it's inconsistent between "t" and "__t".


================
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;
     }
----------------
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.


================
Comment at: libcxx/test/libcxx/ranges/range.access/range.prim/data.incomplete.verify.cpp:23
-  // expected-error@*:* {{no matching function for call}}
-  std::ranges::data(arr);
-}
----------------
This is calling `ranges::data` on a pointer variable. The expected-error here is surprising, but I didn't look into it.


================
Comment at: libcxx/test/libcxx/ranges/range.access/range.prim/data.incomplete.verify.cpp:35
-  // expected-error@*:* {{no matching function for call}}
-  std::ranges::data(arr);
-}
----------------
If this were `std::move(arr)`, then it would be required to SFINAE away, not hard-error. But it's not.


================
Comment at: libcxx/test/libcxx/ranges/range.access/range.prim/empty.incomplete.verify.cpp:24
-  // expected-error@*:* {{attempt to use a deleted function}}
-  std::ranges::begin(arr);
-}
----------------
Every test in this file tests `ranges::begin`, when the test file is named `empty.incomplete.verify.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115838