[libcxx-commits] [PATCH] D102135: [libcxx][ranges] adds _`copyable-box`_

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/include/__ranges/copyable_box.h:40-41
+
+template<class _Tp>
+concept __copy_constructible_object = copy_constructible<_Tp> && is_object_v<_Tp>;
+
----------------
ldionne wrote:
> cjdb wrote:
> > Quuxplusone wrote:
> > > ldionne wrote:
> > > > cjdb wrote:
> > > > > Please move this to `<concepts>`: I plan to use this elsewhere! Also, it should be:
> > > > > ```
> > > > > template<class _Tp>
> > > > > concept destructible_object = destructible<_Tp> && is_object_v<_Tp>;
> > > > > 
> > > > > template<class _Tp>
> > > > > concept move_constructible_object = move_constructible<_Tp> && destructible_object<_Tp>;
> > > > > 
> > > > > template<class _Tp>
> > > > > concept copy_constructible_object = copy_constructible<_Tp> && move_constructible_object<_Tp>;
> > > > > ```
> > > > > (If we need `constructible_object_from`, I'll add that later.)
> > > > I suggest that we instead move it to `<concepts>` when we have that second use for it. As it is, it's really an implementation detail of `copyable_box`.
> > > > 
> > > > Also, http://eel.is/c++draft/range.copy.wrap#1.1 says:
> > > > 
> > > > > `copyable-box<T>` constrains its type parameter `T` with `copy_­constructible<T> && is_­object_­v<T>`.
> > > > 
> > > > `copy_­constructible<T>` already includes `destructible<T>` transitively (via `constructible_from`, at least).
> > > +1 @ldionne. Also, there's certainly no need to introduce multiple subsuming concepts here. Subsumption is a tool for solving a problem (namely, concept overloading); we should use the tool if-and-only-if we are solving that problem (or of course if the paper standard is mandating us to do so).
> > Sure, I'll ask Zoe to move it in D103056, or when `single_view` is implemented.
> > 
> > > `copy_­constructible<T>` already includes `destructible<T>` transitively (via `constructible_from`, at least).
> > 
> > The point of my implementation is to create a subsumption hierarchy that doesn't eventually create a conflict between the groupings `destructible<T> && is_object_v<T>`, `move_constructible<T> && is_object_v<T>`, and `copy_constructible<T> && is_object_v<T>`. I don't want there to be a competing `__move_constructible_object` later on down the line, as I suspect there inevitably will be. Designing concepts is tricky business, and putting `is_object_v` here is arbitrary. It makes more sense to put it on the base concept and grow out, which is why `destructible` is at the base of all `constructible` concepts.
> If the Standard had used some exposition-only concept `__copy_constructible_object` to constrain `F` in `transform_view`, then I'd agree strongly. As it is, I'm lukewarm. Let's lift it once we have a couple uses for it.
If the goalpost is going to be moved, let's get rid of it here too. The standard doesn't say anything about `__copy_constructible_object` at all, and if we're going to limit ourselves to only standard-provided exposition-only detail concepts: you're breaking ranks here.

In other words: this decision is arbitrary and inconsistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135