[libcxx-commits] [PATCH] D101079: [libcxx][ranges] Add ranges::size CPO.

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 22 11:51:58 PDT 2021


cjdb added inline comments.


================
Comment at: libcxx/include/__ranges/size.h:82
+  template <class _Tp, size_t _Sz>
+  [[nodiscard]] constexpr size_t operator()(_Tp (&)[_Sz]) const noexcept {
+    return _Sz;
----------------
> Otherwise, if `T` is an array type, `ranges​::​size(E)` is expression-equivalent to `decay-copy(extent_­v<T>)`.

This needs to be changed to support all bounded arrays, not just lvalue bounded arrays (especially in the wake of the deleted overload).


================
Comment at: libcxx/include/__ranges/size.h:105
+
+  size_t operator()(auto &&) const = delete;
+};
----------------
Quuxplusone wrote:
> zoecarver wrote:
> > zoecarver wrote:
> > > cjdb wrote:
> > > > Quuxplusone wrote:
> > > > > Could you explain why this `=delete`'d overload is necessary?
> > > > > I see how it is //observable// ( https://godbolt.org/z/PGxzf4jGz ) and I assume it is //permitted//, but I don't see why it's //necessary// in libc++. Other implementors (GCC, MSVC) get away without it.
> > > > It provides good intrinsic documentation. @zoecarver can you please contrast the diagnostic with/without this deletion, and maybe upload them to a GitHub gist?
> > > This is a so called poison pill. I could be persuaded to remove it, but I think it makes the error message nicer for users. I'd like to point out that other implementations //do// use it. 
> > @cjdb I didn't see your comments when I made mine :)
> > 
> > Anyway, here's a gist: https://gist.github.com/zoecarver/594015f42664e3daa7ab7c3978027682
> > 
> > I'm now less convinced that deleting this overload is the correct way to go. 
> @zoecarver: No. The "poison pill" is the thing on lines 50-51. This is a plain old deleted overload of `operator()`, and GCC/MSVC seem not to use it. Check out the godbolt where the difference is observable:  https://godbolt.org/z/PGxzf4jGz and compare it locally versus your implementation.
Admittedly the deleted function is less useful in `ranges::size` than in `ranges::begin` because `ranges::size` accommodates all array bounded arrays, not just lvalue arrays. I support whichever provides cleaner diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101079



More information about the libcxx-commits mailing list