On Thu, 20 Feb 2020, Jonathan Wakely wrote:
> On 19/02/20 23:53 -0500, Patrick Palka wrote:
> > ... 'subrange-y' view adaptors
> >
> > This implements an interpretation of P1739R4. It's only an "interpretation"
> > becuase AFAICT there is an issue with the paper's wording as-is. So this
> > patch
> > deviates from paper when it comes to the problematic wording.
> >
> > The issue is that the paper's wording for views::take requires that the
> > views::take of a specialization of subrange is not just another subrange,
> > but
> > specifically is the same specialization as the input subrange.
> > But if, say, the input subrange does not model common_range, then the
> > expression
> > in section 6.1
> >
> > T{begin(E), begin(E) + min(size(E), F)}
> >
> > is ill-formed because T -- a specialization of subrange which does not model
> > common_range -- must be constructed with an iterator-sentinel pair and not
> > an
> > iterator-iterator pair. A similar issue arises with the views::take of an
> > iota_view whose value type differs from the type of its bound.
> >
> > So in light of this issue, this patch deviates from the paper's wording here
> > by
> > allowing the views::take of a subrange/iota_view input to be a different
> > specialization than that of the input.
> >
> > Moreover, I found myself needing to define an extra constrained constructor
> > iota_view(iterator_, iterator_) alongside the newly added
> > iota_view(iterator_, sentinel_) constructor, so that the expression
> > in views::take
> >
> > iota_view<ValueType,ValueType>{begin(E), begin(E) + min(size(E), F)}
> >
> > is well-formed in general. Despite these deviations, the intended end
> > result
> > remains the same AFAICT.
>
> Please be sure to report these to the LWG chair address so issues can
> be opened.
Sounds good, I will report them issues probably today or tomorrow.
>
> > @@ -1965,10 +1993,70 @@ namespace views
> >
> > namespace views
> > {
> > + namespace __detail
> > + {
> > + template<typename _Tp>
> > + inline constexpr bool __is_empty_view = false;
> > +
> > + template<typename _Tp>
> > + inline constexpr bool __is_empty_view<empty_view<_Tp>> = true;
> > +
> > + template<typename _Tp>
> > + inline constexpr bool __is_dynamic_span = false;
> > +
> > + template<typename _Tp>
> > + inline constexpr bool __is_dynamic_span<span<_Tp, dynamic_extent>>
> > + = true;
> > +
> > + template<typename _Tp>
> > + inline constexpr bool __is_basic_string_view = false;
> > +
> > + template<typename _CharT, typename _Traits>
> > + inline constexpr bool
> > + __is_basic_string_view<basic_string_view<_CharT, _Traits>> = true;
>
> I think it should be possible to define these without including <span>
> and <string_view>. Either by adding forward declarations of span and
> basic_string_view, which is sufficient to define the variable template
> specializations, or by defining something here and specializing it in
> <span> and <string_view> e.g. in <ranges>:
>
> template<typename _Tp>
> inline constexpr bool __is_viewish = false;
>
> And then in <span> add:
>
> template<typename _Tp>
> extern inline const bool __is_viewish;
> template<typename _Tp>
> inline constexpr bool __is_viewish<span<_Tp>> = true;
>
> The first declaration is needed so that <span> works without including
> <ranges>, and vice versa.
>
> And in <string_view>:
>
> #if __cplusplus > 201703L
> template<typename _Tp>
> extern inline const bool __is_viewish;
> template<typename _Ch, typename _Tr>
> inline constexpr bool __is_viewish<basic_string_view<_Ch, _Tr>>
> = true;
> #endif
>
> That way we don't need to declare span and string_view in <ranges> at
> all. We also don't need two separate variables, one will do. And
> finally, doing it this way allows us to enable this feature for
> experimental::basic_string_view too:
>
> And in <experimental/string_view>:
>
> #if __cplusplus > 201703L
> template<typename _Tp>
> extern inline const bool __is_viewish;
> template<typename _Ch, typename _Tr>
> inline constexpr bool
> __is_viewish<experimental::basic_string_view<_Ch, _Tr>> = true;
> #endif
>
> This last specialization *must* be defined in
> <experimental/string_view> not in <ranges>, because we don't want to
> "leak" the non-reserved "experimental" name into <ranges> when the
> user hasn't explicitly included a TS header.
>
> A better name than "viewish" would be nice, but it does look like we
> can use the same one for span and string_view, since you always treat
> them the same.
Thanks for the detailed explanation. I'll update the patch to use this
design.
