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

P2502 std::generator: Synchronous Coroutine Generator for Ranges #1151

Closed
brycelelbach opened this issue Dec 15, 2021 · 11 comments · Fixed by cplusplus/draft#5695
Closed

P2502 std::generator: Synchronous Coroutine Generator for Ranges #1151

brycelelbach opened this issue Dec 15, 2021 · 11 comments · Fixed by cplusplus/draft#5695
Labels
B1 - focus Bucket 1 as described by P0592: material that is mentioned in this plan. C++23 Targeted at C++23 coroutines Coroutines / Fibers IS Ship vehicle: IS LWG Library plenary-approved Papers approved for inclusion in their target vehicle by plenary vote. ranges std::ranges tentatively-ready-for-plenary Reviewed between meetings; ready for a vote.
Milestone

Comments

@brycelelbach
Copy link

P2502R0 std::generator: Synchronous Coroutine Generator for Ranges (Casey Carter)

@brycelelbach brycelelbach added LEWG Library Evolution coroutines Coroutines / Fibers ranges std::ranges C++23 Targeted at C++23 IS Ship vehicle: IS B1 - focus Bucket 1 as described by P0592: material that is mentioned in this plan. ready-for-library-evolution-meeting-review This paper needs to be discussed at a Library Evolution meeting scheduled-for-library-evolution This paper has been scheduled for one of the groups: LEWG, LEWG Incubator, or a Mailing List review labels Dec 15, 2021
@brycelelbach
Copy link
Author

brycelelbach commented Dec 15, 2021

2021-12-13 Library Evolution Telecon

P2168R3: std::generator

P2502R0: std::generator

2021-12-13 Library Evolution Telecon Minutes

Chair: Bryce Adelstein Lelbach

Champion: Casey Carter

Minute Taker: Ben Craig

Start: 2021-12-13 8:12 Pacific

Does this proposal have:

  • Examples?
    • Yes.
    • Adding before/after examples might be helpful, as the author states that std::generator greatly reduces the amount of code you'd have to write for the examples.
  • Implementation experience?
    • Yes, in third party libraries like CppCoro and Folly, but not in any major Standard Library.
  • Usage experience?
    • Yes, in third party libraries like CppCoro and Folly, but not in any major Standard Library.
  • Deployment experience?
    • Yes, in third party libraries like CppCoro and Folly, but not in any major Standard Library.
  • Performance considerations?
    • Yes.
  • Discussion of prior art?
    • Yes. Direct descendant of P2168, which builds on experience from third party libraries.
  • Changes Library Evolution previously requested?
    • The paper considers and attempts to address open questions that arose during previous discussions.
  • Wording?
    • Yes.
  • Feature test macro?
    • Yes.

Template parameters in P2168:

template <class Ref,
          common_reference_with<Ref> Value = remove_cvref_t<Ref>,
          class Allocator = void>
class generator;

Template parameters in P2502:

template <class T,
          class Allocator = void,
          class U = void>
class generator;

where:

using Value = conditional_t<is_void_v<U>, remove_cvref_t<T>, U>;}
using Reference = conditional_t<is_void_v<U>,
                                conditional_t<is_reference_v<T>, T, const T&>,
                                T>;
using Yielded = conditional_t<is_reference_v<Reference>, Reference, const Reference&>;

The above are currently exposition only types, but some expressed a desire for them to be a part of the public interface.

What should the reference type for generator<T> be when T is not a reference type? The paper currently proposes const T&; some suggest T&&.

Consider generator<string> - string&& would be odd reference type for a range. But for generator<unique_ptr<T>>, unique_ptr<T> const& would be odd.

Add a table to generator<meow> showing examples of what the reference type would be for some common cases of T, such as string, string&&, unique_ptr<T>.

Should the allocator parameter come before the U parameter?

Is there any precedent in the Standard Library for an allocator template parameter that's not the last template parameter?

There's some suggestion that supporting recursive generators has an overhead for non-recursive use cases. Should generator support recursive generators, or should we add a separate type for that later?

Open Questions:

  • Default reference type for generator<T>.
  • generator support for recursion.
  • Position of generator's allocator template parameter.
  • Header for generator.

Topics to Polls:

  • Position of the allocator template parameter.
  • Header for generator.

POLL: Make generator's allocator template parameter the last template parameter.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
2 5 7 5 0

Attendance: 27

# of Authors: 1

Author Position: WA

Outcome: No consensus.

This topic needs further discussion in future revisions.

POLL: Put generator in <ranges> instead of adding a new <generator> header.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
3 7 2 8 1

Attendance: 26

# of Authors: 1

Author Position: WF

Outcome: No consensus.

This topic needs further discussion in future revisions.

End: 9:30

Summary

Today we discussed P2502, an updated proposal for std::generator. This new revision proposes a new approach for template parameterization, which seems to address some of the issues identified with prior revisions. It was received well.

We discussed some remaining open questions, notably:

  • The default reference type for generator<T>.
  • generator support for recursion.
  • The position of generator's allocator template parameter.
  • The header that generator should live in.

We polled on the position of generator's allocator template parameter and the header that generator should live in. Both polls were inconclusive; further discussion is needed.

Outcome

We will continue design review of P2502 generator at our next telecon.

@brycelelbach
Copy link
Author

P2502R0: std::generator

P2502R0: std::generator

2021-12-21 Library Evolution Telecon Minutes

Chair: Bryce Adelstein Lelbach

Champion: Casey Carter

Minute Taker: Ben Craig

Start: 2021-12-21 13:21 Eastern

Options for reference type for generator<T> where T isn't a reference type:

  • const T& (Casey)
  • T&& (Mathias)
  • Some cleverness to allow us to do either, e.g. moving() (Tomasz)
  • Make generator<T> ill-formed (nuclear option)

What do existing implementations do for reference type of generator<T>:

  • CppCoro: T&.
  • folly: T&.
  • ranges v3: T.
  • MSVC's experimental generator: const T&.

POLL: generator<T> (where T isn't a reference type) should have a reference type of T&& instead of const T&.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
2 3 3 4 4

Attendance: 21

# of Authors: 1

Author Position: 1x WA

Outcome: No consensus.

We'll retake the poll as some may have misunderstood it.

If the next poll is inconclusive, poll the nuclear option.

POLL: generator<T> (where T isn't a reference type) should have a reference type of const T&.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
5 4 3 2 2

Attendance: 21

# of Authors: 1

Author Position: 1x SF

Outcome: No consensus.

POLL: generator<T> (where T isn't a reference type) should be ill-formed in its first release.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
0 7 6 3 0

Attendance: 21

# of Authors: 1

Author Position: 1x WF

Outcome: No consensus.

POLL: The first generator we release must support yielding nested generators.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
1 1 6 9 1

Attendance: 21

# of Authors: 1

Author Position: 1x N

Outcome: Weak consensus against.

End: 14:33

Summary

Today we continued our discussion of the new generator proposal, P2502. Since our last review, a small group of stakeholders met to discussion the proposal, but unfortunately became deadlocked on one major question: for the simplest spelling of a generator, which we expect to be most commonly used, what should the reference type generator<T>'s ranges and iterators be when T is not a reference type?

There are five different options:

  1. T: This is a performance pitfall, as you'll make a copy every time an iterator is dereferenced. This was proposed in earlier generator proposals, but no one is currently advocating for this.
  2. const T&: This avoids making a copy on every dereference, and is what is currently proposed. However, this approach is problematic with move-only types such as unique_ptr that you may wish to move out of a generator iterator. We polled this option and the outcome was inconclusive.
  3. T&: This seems a bit nonsensical and hazardous, but it appears to be what at least some existing implementations do.
  4. T&&: This would avoid copies in the common case and work for move-only types. However, it presents a number of safety and correctness concerns; what happens if you move the value on the first dereference, and then dereference again later? We polled this option and the outcome was inconclusive.
  5. const T& by default, but you can convert it to a range of T&&. This would presumably require some cleverness and a flag for bookkeeping. We didn't poll this option because it was a new idea that required further evaluation.
  6. Make generator<T> ill-formed when T is not a reference type. If we can't agree on the semantics, then we could just require users to be specific about what they want. We polled this option, and while we didn't have consensus, there was some indication that we may be able to eventually get to consensus in favor of this.

Regardless of what we decide for this case, you would still be able to specify what you want, e.g.generator<T&&>, generator<T const&>, or generator<string_view, void, string>.

We also briefly discussed whether or not we needed the first generator type that we ship in the Standard Library to support yielding nested generators. Some are concerned about performance issues that may arise from yielding nested generators, as compilers may not be able to optimize such patterns. Others felt that a generator that doesn't support yielding nested generators is easy to write and may not be worth standardizing. We had consensus that it was not a hard requirement for the first generator we ship to support yielding nested generators. The authors should explore whether removing this requirement would simplify the proposal.

Finally, we had a frank discussion about the likelihood of getting any generator type into C++23. The chair pointed out that it is the end of December 2021 and C++23 has to be feature complete by February 2022. If we want something in C++23, we'll have to be willing to compromise and limit the scope of the feature. Even if we do that, it seems unlikely that we'll be able to land something before the feature complete date.

However, as coroutines library support is a plenary approved priority, we will continue working on it in January. We'll use the last two Library Evolution telecons of January to review the next revision of the proposal. If we do decide to ship something in C++23, we would have to run a short week-long electronic poll to confirm that decision before the February 2022 deadline.

Outcome

Bring a revision of P2502R0 (std::generator), with the guidance below, to Library Evolution for further design review.

  • Find a solution that can get consensus for the default reference type for generator<T> when T is not a reference type.
  • Explore whether dropping support for yielding nested generators simplifies the proposal.
  • Document existing field experience, and get more of it.

@brycelelbach brycelelbach added size - large paper size estimate size - medium paper size estimate and removed size - large paper size estimate labels Dec 22, 2021
@jensmaurer jensmaurer added this to the 2022-telecon milestone Jan 1, 2022
@brycelelbach brycelelbach added ready-for-library-evolution-electronic-poll This paper needs to undergo a Library Evolution electronic poll and removed ready-for-library-evolution-meeting-review This paper needs to be discussed at a Library Evolution meeting labels Feb 1, 2022
@cor3ntin
Copy link

cor3ntin commented Feb 16, 2022

2022-01-25 Library Evolution Telecon

P2502R1: generator

P2529R0: generator should have T&& reference_type

2022-01-25 Library Evolution Telecon Minutes

Chair: Gašper Ažman

Champion: Casey Carter (P2502) and Mathias Stearn (P2529)

Minute Taker: Bronek Kozicki

Poll: Rename generator to recursive_generator.

SF F N A SA
1 6 5 10 0

Name stays.

Poll: We are OK with generator<T> behaving as generator<T const&> (P2502R1)

SF F N A SA
1 10 3 6 3

Poll: We are OK with generator<T> behaving as generator<T&&> (D2529R0)

SF F N A SA
8 7 4 3 2

Poll: We are OK with generator<T> being ill-formed. (the nuclear option)

SF F N A SA
4 4 5 5 6

Poll: Forward P2502R1 to LWG (with generator<T> behaving exactly as generator<T&&> - D2529R0) for inclusion in C++23 as a P1 item (to be confirmed by electronic polling).

SF F N A SA
16 4 0 2 0

Next Steps

Take an electronic poll to send P2502R1 (generator<T>) with generator<T> behaving exactly as generator<T&&> (D2529R0) to Library for C++23, classified as B1 - Priority.

@cor3ntin cor3ntin added LWG Library and removed LEWG Library Evolution ready-for-library-evolution-electronic-poll This paper needs to undergo a Library Evolution electronic poll scheduled-for-library-evolution This paper has been scheduled for one of the groups: LEWG, LEWG Incubator, or a Mailing List review labels Feb 16, 2022
@wg21bot
Copy link
Collaborator

wg21bot commented Feb 22, 2022

P2502R1 std::generator: Synchronous Coroutine Generator for Ranges (Casey Carter)

@brycelelbach brycelelbach added the lwg-pending LWG Chair needs to disposition label Feb 23, 2022
@brycelelbach
Copy link
Author

2022-01 Library Evolution Electronic Poll Outcomes

Send [P2502R1] (std::generator) to Library Working Group for C++23, classified as a focus ([P0592R4] bucket 1 item)

SF F N A SA
18 11 1 1 1

Consensus in favor.

@brycelelbach
Copy link
Author

@JeffGarland how confident are we that we'll get to this in time for C++23?

@JeffGarland
Copy link
Member

It's obviously a high priority, but I've asked @dietmarkuehl if he could run some pre-review outside of regular LWG group time to make refinements before we see it in full group. I believe he will try to pick that up after ACCU (currently ongoing) - we should see if @CaseyCarter could participate

@brycelelbach
Copy link
Author

brycelelbach commented Apr 8, 2022 via email

@CaseyCarter
Copy link

we should see if @CaseyCarter could participate

Yes, absolutely - I've been trying to make time for an editorial pass before LWG review, this will make it easier.

@JeffGarland
Copy link
Member

JeffGarland commented Jun 3, 2022

LWG completed a number of small group and 2 large group reviews on 2022-06-03 -- approving for C++23.

Notes from 04-26 small group https://wiki.edg.com/bin/view/Wg21telecons2022/P2502-20220426
Notes from 05-03 small group https://wiki.edg.com/bin/view/Wg21telecons2022/P2502-20220503
Notes from 05-27 full group https://wiki.edg.com/bin/view/Wg21telecons2022/P2502-20220527
Notes from 0603 full group - TBA

Poll: Adopt P2502R2 std::generator for C++23

F N A
11 0 0

@JeffGarland JeffGarland added tentatively-ready-for-plenary Reviewed between meetings; ready for a vote. and removed lwg-pending LWG Chair needs to disposition size - medium paper size estimate labels Jun 3, 2022
@wg21bot
Copy link
Collaborator

wg21bot commented Jun 24, 2022

P2502R2 std::generator: Synchronous Coroutine Generator for Ranges (Casey Carter)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B1 - focus Bucket 1 as described by P0592: material that is mentioned in this plan. C++23 Targeted at C++23 coroutines Coroutines / Fibers IS Ship vehicle: IS LWG Library plenary-approved Papers approved for inclusion in their target vehicle by plenary vote. ranges std::ranges tentatively-ready-for-plenary Reviewed between meetings; ready for a vote.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants