ericniebler / stl2

LaTeX and Markdown source for the Ranges TS/STL2 and associated proposals
88 stars 8 forks source link

const-ness of Readable types should not effect associated types #514

Open ericniebler opened 7 years ago

ericniebler commented 7 years ago

The current state of affairs with Readable is that dereferencing a const-qualified object may return a different type than dereferencing a non-const-qualified object. This is a usability nightmare.

It was done that way partly on accident and partly to support "readable" types like optional. I think we should cut optional loose and focus Readable on types that logically represent an indirection.

Proposed Resolution

Change [incrementable.traits]/p2 as follows:

2 -- The type iter_difference_t<I> denotes

2.1 -- incrementable_traits<remove_cvref_t<I>>::difference_type if iterator_traits<remove_cvref_t<I>> names a specialization generated from the primary template, and

2.2 -- iterator_traits<remove_cvref_t<I>>::difference_type otherwise.

Change [readable.traits]/p2 as follows:

2 -- The type iter_value_t<I> denotes

2.1 -- readable_traits<remove_cvref_t<I>>::value_type if iterator_traits<remove_cvref_t<I>> names a specialization generated from the primary template, and

2.2 -- iterator_traits<remove_cvref_t<I>>::value_type otherwise.

[Change [iterator.concept.readable]/p1 as follows:]{.ednote}

1 -- Types that are indirectly readable by applying operator* model the readable concept, including pointers, smart pointers, and iterators.

template<class In>
- concept readable =
-   requires {
+ concept __readable =
+   requires (const In in) {
      typename iter_value_t<In>;
      typename iter_reference_t<In>;
      typename iter_rvalue_reference_t<In>;
+     { *in } -> same_as<iter_reference_t<In>>;
+     { iter_move(in) } -> same_as<iter_rvalue_reference_t<In>>;
    } &&
    common_reference_with<iter_reference_t<In>&&, iter_value_t<In>&> &&
    common_reference_with<iter_reference_t<In>&&, iter_rvalue_reference_t<In>&&> &&
    common_reference_with<iter_rvalue_reference_t<In>&&, const iter_value_t<In>&>;

+template<class In>
+  concept readable =
+    __readable<remove_cvref_t<In>>;
CaseyCarter commented 7 years ago

Further, Readable<T> currently imposes no requirements on the expression *declval<U>() where U differs from T&. Consequently, e.g. ForwardIterator<T> only requires that * is valid for an lvalue T. One may not dereference an lvalue const T, prvalue T, etc.

CaseyCarter commented 7 years ago

WIP

Open question: to what extent can we harmonize Readable with Writable here? Writable performs gymnastics to apply requirements to both T&& and T& without requiring const, presumably to allow optional to satisfy Writable. We no longer feel this corner case need be valid, so Writable can potentially be simplified to a form that uses implicit expression variants. Notably, the majority (all?) of the usages in the TS supply a Semiregular type for the first parameter of Writable.

 template <class In>
-concept bool Readable = 
+concept bool __ReadableTypes = // \expos
   requires {
     typename value_type_t<In>;
     typename reference_t<In>;
     typename rvalue_reference_t<In>;
   } &&
   CommonReference<reference_t<In>&&, value_type_t<In>&> &&
   CommonReference<reference_t<In>&&, rvalue_reference_t<In>&&> &&
   CommonReference<rvalue_reference_t<In>&&, const value_type_t<In>&>;

+template <class In>
+concept bool Readable = 
+  __ReadableTypes<remove_reference_t<In>> &&
+  __ReadableTypes<const remove_reference_t<In>> &&
+  Same<
+    value_type_t<remove_reference_t<In>>, 
+    value_type_t<const remove_reference_t<In>>> &&
+  Same<
+    reference_t<remove_reference_t<In>>, 
+    reference_t<const remove_reference_t<In>>> &&
+  Same<
+    rvalue_reference_t<remove_reference_t<In>>, 
+    const rvalue_reference_t<In>>>;

Note that this partial PR makes Readable potentially too expensive to compile by more-than-doubling the number of checked requirements.

ericniebler commented 5 years ago

Might be better to simply add the requirements in prose without requiring an implementation to diagnose failures.

ericniebler commented 5 years ago

I think part of the problem here is that we're trying to be overly general with this concept by trying to support types that do not represent a logical or physical indirection (and doing a half-assed job of it). It occurred to me yesterday that part of the problem is due to a bad name. The STL doesn't care about readability and writability. It cares about indirect readability and writability. This is reflected in the names of the other concepts (see indirectly_swappable).

So possibly a fix is to rename readable and writable to indirectly_readable and indirectly_writable and re-express them to assume indirection; that is, top-level const should be ignored. We could then use expression variations to fix this.

This is all idle musings at this point. Next would be to try it and see what breaks.

ericniebler commented 5 years ago

After perusing the current working paper, there are output iterators that have non-const-qualified operator* members, so this change probably won't fly for writable. However, all the input iterators have const-qualified operator* members -- with the minor exception of the exposition-only istreambuf_iterator::proxy::operator*, which can trivially have const added to its member.

ericniebler commented 5 years ago

Writable performs gymnastics to apply requirements to both T&& and T& without requiring const, presumably to allow optional to satisfy Writable.

No, the gymnastics in writable are so that an iterator that returns, say, a std::string by prvalue doesn't satisfy writable by accident. Because nobody ref-qualifies their assignment operators (dammit). The gymnastic are necessary.

CaseyCarter commented 5 years ago

the gymnastics in writable are so

We're talking about completely different gymnastics. If we were happy to require writable's expressions to work with const-qualified iterators we could formulate it as:

template<class Out, class T>
  concept writable =
    requires(const Out& o, T&& t) {
      *o = std::forward<T>(t); // not required to be equality-preserving
      const_cast<const iter_reference_t<Out>&&>(*o) =
std::forward<T>(t); // not required to be equality-preserving
    };

and let implicit expression variations (IEVs) cover the rvalue cases. We didn't want to do that, so we have distinct "lvalue" and "rvalue" flavors of each of the two required expressions above. These are the gymnastics I refer to. You are referring to the two const_cast expressions, which exist to deal with the fact that the default assignment operators for class types work with rvalue left-hand-sides.

(Concepting is hard.)

ericniebler commented 5 years ago

We're talking about completely different gymnastics.

Ah.

If we were happy to require writable's expressions to work with const-qualified iterators...

No, I don't think we are. Lots of output iterators (many in the standard even) have non-const qualified operator* members. Let's not go there.