`const subrange<I,S,[un]sized>` is not a forwarding-range
ericniebler opened this issue · 10 comments
Given the current wording, the following will not compile:
int *p = ranges::cbegin(ranges::subrange<int*>{});
That's because subrange
defines its rvalue begin
overload as:
friend constexpr I begin(subrange&& r) { return r.begin(); }
This also has the problem that any type that happens to have subrange
as an associated namespace (like split_view<subrange<...>>
will find this overload and attempt a conversion from split_view
to subrange&&
, which could possibly cause weirdness.
Better to define the overload as:
template<class A, class B> concept same-ish = Same<A const, B const>;
template<class I, ...>
struct subrange {
...
friend constexpr I begin(same-ish<subrange> auto && r) { return r.begin(); }
...
};
It also fixes the problem of a type deriving from subrange
and appearing to model forwarding-range
when in fact it might not.
We also want to give iota_view
a similar treatment (see #575). ref_view
's begin
/end
should take by Same<ref_view> auto r
.
Finally, the poison-pill overloads for std::initializer_list
are wrong. Should be:
-template<class T> void begin(initializer_list<T> &&) = delete;
+template<class T> void begin(initializer_list<T>) = delete;
Likewise for end
.
Proposed Resolution
In [range.access.begin]/p1.3, change the poison-pill overloads as follows:
template<class T> void begin(T&&) = delete;
-template<class T> void begin(initializer_list<T>&&) = delete;
+template<class T> void begin(initializer_list<T>) = delete;
To the synopsis in [range.subrange]/p1, add:
template<class A, class B>
concept same-ish = // exposition only
same_as<A const, B const>;
In the class synopsis of subrange
(same section), change the begin
/end
friend functions to be as follows:
friend constexpr I begin(same-ish<subrange> auto && r) { return r.begin(); }
friend constexpr S end(same-ish<subrange> auto && r) { return r.end(); }
In the synopsis of ref_view
in [range.ref.view]/p1, change the begin
/end
friend functions as follows (editors note: the use of same_as
here instead of same-ish
is intentional):
-friend constexpr iterator_t<R> begin(ref_view r)
+friend constexpr iterator_t<R> begin(same_as<ref_view> auto r)
{ return r.begin(); }
-friend constexpr sentinel_t<R> end(ref_view r)
+friend constexpr sentinel_t<R> end(same_as<ref_view> auto r)
{ return r.end(); }
To the class synopsis of iota_view
in [range.iota.view], add the following begin
/end
friend functions:
friend constexpr W begin(same-ish<iota_view> auto && r) { return r.begin(); }
friend constexpr auto end(same-ish<iota_view> auto && r) { return r.end(); }
Does this formulation work, @ericniebler?
friend constexpr I begin(same_as<subrange> auto r) { return r.begin(); }
Or even:
friend constexpr I begin(subrange r) { return r.begin(); }
It also fixes the problem of a type deriving from subrange and appearing to model forwarding-range when in fact it might not.
This one I don't think is a problem, since the poison-pill would be a better match for the derived types: https://godbolt.org/z/JK4KQ4
For the record: I vastly prefer the simple "take the forwarding-range
by value" formulation to the same_as<foo> auto
hack.
Yeah I started writing that comment before I realized that the by-value version actually fixes the problem that Eric was concerned about. I'm not sure there is any advantage to the template version?
If the begin
function in question is a function and not a function template, then whenever it is found by ADL, the compiler will consider conversation sequences to its argument. That's what I was saying above about potential weirdness.
UPDATE: Or does the poison pill fix that also? I guess it will throw all the overloads in a big pot and try them all, and only fail if the poison pill is selected. "Trying them all" might involve looking for a conversion sequence to subrange
and that has the potential to introduce hard errors or constraint recursion. Call me paranoid.
Yeah the poison pill fixes that too. Non-template version, both derived and convertible: https://godbolt.org/z/QUhHdI
If this is so concerning, we should just introduce an explicit-opt-in variable template instead of doing overload resolution for an explicit-opt-in non-member. Wouldn't that compile faster anyway?
The issue is not picking that overload. It's the process of determining whether that overload is viable, even if we'll never end up picking it anyway. That can necessitate template instantiations and headaches, and is definitely permitted (if not required).
Thanks, @tcanens. This is the concern, yes. It's not an issue for function templates. So another potential fix would be to define the subrange
begin
function as:
template<class It, class Sent, subrange_kind Kind>
constexpr It begin( subrange<It, Sent, Kind> rng )
{
return rng.begin();
}
... and put it either at namespace scope or make it a friend in a private (non-dependent) base of subrange
.
EDIT: I suppose we could document it as a member of namespace std::ranges
alongside subrange
. I think an implementation that makes it a friend in a private base would be conforming. Users can't (?) tell the difference in a conforming program since they are not allowed to take the address of functions in std::
.
EDIT 2: Making it a function template in namespace std::ranges
puts it in the same declarative region as the ranges::begin
CPO, which is obviously ill-formed. But we already have this "problem": as a friend function in subrange
it collides with ranges::begin
. We have been waving our hands about that, trusting implementors to solve the problem in the obvious[*] way: by putting it in a different declarative region where ADL will still find it.
[*]: Nothing about this is obvious.
I would argue that if:
friend constexpr I begin(subrange r) { return r.begin(); }
isn't considered good enough, then we should rethink non-member begin
as the customization point and seriously consider just a variable template type trait. This is already super complicated!
My concerns about accidental opt-in to forwarding-range
have been alleviated, but I'm not sure what the relative benefit is of the non-member+poison-pill design vs just a type trait? The current approach seems more compile-expensive?
Fair.
Though I should say that very few people will ever be troubled to opt-in to this, and that if they do, they will probably do so with a namespace-scoped begin
function or function template, and that even if they write a non-template friend thing, they're still extremely unlikely to run into problems due to bad conversion sequences. My reason for using begin
this way was to avoid creating Yet Another Trait, and I don't love traits.
what the relative benefit is of the non-member+poison-pill design vs just a type trait?
We have the begin
CPO and the poison-pill anyway. Why add a trait when we can just use what we have? This is my rationale. I can see the other side of this also.
In my NB comment, I suggested:
template <... >
struct subrange {
friend constexpr I begin(same_as<subrange> auto r) { return r.begin(); }
friend constexpr S end(same_as<subrange> auto r) { return r.end(); }
};
I think this is actually a mistake, particularly for end
. If r
is an xvalue, and if I
is a move-only iterator, then simply calling end
will destroy the begin position.
I think I had it right the first time with the same-ish
<subrange> auto && r
formulation. I need to follow up with lwgchair and maybe with Barry to correct the draft NB comments.