ericniebler / stl2

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

shared_ptr<int>& is not Readable #594

Open ericniebler opened 5 years ago

ericniebler commented 5 years ago

In the current spec, shared_ptr<int> is Readable, but shared_ptr<int>& is not. That is because readable_traits is not stripping top-level references before testing for nested typedefs.

Either readable_traits should see through cv- and ref-qualifiers, or else the Readable concept should strip top-level references when building the iter_value_t associated type (e.g., iter_value_t<remove_reference_t<In>>).

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.

ericniebler commented 5 years ago

~Proposed Resolution~ SUPERCEDED, SEE ABOVE

Change [iterator.concept.readable] as follows:

 template<class In>
   concept readable =
     requires {
-      typename iter_value_t<In>;
+      typename iter_value_t<remove_reference_t<In>>;
       typename iter_reference_t<In>;
       typename iter_rvalue_reference_t<In>;
     } &&
-    common_reference_with<iter_reference_t<In>&&, iter_value_t<In>&> &&
+    common_reference_with<iter_reference_t<In>&&, iter_value_t<remove_reference_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>&>;
+    common_reference_with<iter_rvalue_reference_t<In>&&,
+                          const iter_value_t<remove_reference_t<In>>&>;
ericniebler commented 5 years ago

@CaseyCarter for your review.

CaseyCarter commented 5 years ago

I've been trying to convince myself that this is the top of slippery slope that will lead to people writing reference confusion bugs a la void foo(destructible auto&& x) (when given an lvalue, this requires a reference type to be destructible when the programmer probably intended to require the referent to be destructible), but I don't see that happening with the iterator concepts and traits as it easily can with the `structibles. So yes, I agree this is an issue we can solve without breaking the design.

I think the PR is fine as well.