Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

US276 24.03.1 p1.3 begin/end friends taking rvalues #272

Closed
wg21bot opened this issue Oct 24, 2019 · 8 comments · Fixed by cplusplus/draft#3486
Closed

US276 24.03.1 p1.3 begin/end friends taking rvalues #272

wg21bot opened this issue Oct 24, 2019 · 8 comments · Fixed by cplusplus/draft#3486
Labels
accepted LWG Library ranges std::ranges
Projects
Milestone

Comments

@wg21bot
Copy link
Collaborator

wg21bot commented Oct 24, 2019

24.03.1
24.3.2
24.5.3
24.7.3.1
24.6.3.2
several

Several of the range views define non-template friend function begin/end overloads taking rvalues to satisfy the exposition-only forwarding-range concept. These have a couple of problems. First, the ones for subrange take subrange&&. That means that a const rvalue subrange fails to satisfy forwarding-range, which causes cbegin(subrange{...}) to be ill-formed.

The bigger problem is that since these functions are non-templates, whenever they get added to the overload set, the compiler will try conversions to these types (subrange, ref_view). The attempted conversions could lead to errors in theory.

Finally, class iota_view has iterator that can safely outlive the view that created them, so it too should be given begin/end friend functions that accept rvalues following the same pattern.

Proposed change:
In [range.access.begin]/p1.3, change the poison-pill overloads from:

template<class T> void begin(T&&) = delete;
template/<class T> void begin(initializer_list<T>&&) = delete;

...to:

template<class T> void begin(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 from:

friend constexpr I begin(subrange&& r) { return r.begin(); }
friend constexpr S end(subrange&& r) { return r.end(); }

...to:

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 from this:

friend constexpr iterator_t<R> begin(ref_view r)
{ return r.begin(); }
friend constexpr sentinel_t<R> end(ref_view r)
{ return r.end(); }

...to this (editors note: the use of same_as here instead of same-ish is intentional; likewise for the use of pass-by-value):

friend constexpr iterator_t<R> begin(same_as<ref_view> auto r)
{ return r.begin(); }
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(); }

See ericniebler/stl2#592.

@wg21bot wg21bot added the LWG Library label Oct 24, 2019
@jensmaurer jensmaurer added the ranges std::ranges label Oct 25, 2019
@jwakely
Copy link
Member

jwakely commented Nov 4, 2019

N.B. The changes for [range.ref.view] should change to same_as<ref_view> auto not same_as auto.

@jensmaurer
Copy link
Member

@jwakely : Transcription error; fixed.

@jensmaurer jensmaurer added this to Inbox in LWG Nov 7, 2019
@mclow
Copy link

mclow commented Nov 8, 2019

LWG Friday morning in Belfast. This should be solved by P1664 (adopted) and P1870 (not adopted yet)

@mclow
Copy link

mclow commented Nov 8, 2019

LWG Friday afternoon in Belfast. Accept P1870R1

@mclow mclow moved this from Inbox to Accepted in LWG Nov 8, 2019
@mclow
Copy link

mclow commented Jan 21, 2020

P1870R1 was adopted in Belfast.
P1664 was not (and an updated version is targeting C++23)

@timsong-cpp
Copy link

We don't need P1664 to resolve this comment. P1870 is sufficient.

@JeffGarland
Copy link
Member

Reviewed by full group in Prague on Monday afternoon. Accepted with modifications and resolved in Belfast with the adoption of P1870R1.

@jensmaurer
Copy link
Member

Accepted with modification. See paper P1870R1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted LWG Library ranges std::ranges
Projects
No open projects
LWG
Accepted
Development

Successfully merging a pull request may close this issue.

6 participants