cplusplus / nbballot

Handling of NB comments in response to ballots
14 stars 4 forks source link

US 46-107 25.5.7.1 [counted.iterator] Too many iterator increments #523

Closed wg21bot closed 1 year ago

wg21bot commented 1 year ago

r | take(n) currently advances the underlying range’s iterator n times past r.begin(), when it really only needs to advance n-1 times. This can lead to surprising and dysfunctional behavior in some contexts that needs to be addressed.

Proposed change:

Adopt P2406 or an alternative solution in this space.

brycelelbach commented 1 year ago

The proposed resolution is to adopt P2406 https://github.com/cplusplus/papers/issues/1076

inbal2l commented 1 year ago

Scheduled for SG9's meeting in Kona, first session (08:00-9:45): https://wiki.edg.com/bin/view/Wg21kona2022/SG9

inbal2l commented 1 year ago

Reviewed by SG9 at Kona2022 meeting (Full Minutes).

Polls

POLL: We support the need to solve the issue brought up on NB comment “US#523: US 46-107 25.5.7.1 [counted.iterator] Too many iterator increments”.

SF F N A SA
5 5 0 0 0

Attendance: 18 (10 on-site, 8 online)

# of Authors: 1

Author’s position: SF

Outcome: Unanimous consensus in favor

__POLL: We support applying the solution in the paper “P2578R0 Block eager input (non-forward) iterators from counted_iterator” for C++23.__

SF F N A SA
0 1 1 5 4

Attendance: 19 (11 on-site, 8 remote)

# of Authors: 1

Author’s position: F

Outcome: Consensus against

__POLL: We support applying the solution in the paper “P2406R0 Fix counted_iterator interaction with input iterators” for C++23.__

SF F N A SA
1 0 0 3 6

Attendance: 19 (11 on-site, 8 remote)

# of Authors: 1

Author’s position: SF

Outcome: Consensus against

Summary

We started the discussion by identifying the problem. There was an agreement of the importance of the issue, but the proposed solution was not accepted.

SG9 agrees with the importance of fixing this issue but does not support the changes currently proposed in the papers (blocking, lazy versions). I would recommend sending the different alternatives in the reflector to start a discussion and get guidance from the group on possible alternative resolutions.

The issue was forwarded to LEWG.

inbal2l commented 1 year ago

The updated paper got additional design feedback from SG9 (2023-01-02 Minutes)

Forwarded to LEWG for C++23 with the recommended design options (Options 2, and I), for C++23 (to be confirmed by an electronic poll / Issaquah 2023 meeting review)

brycelelbach commented 1 year ago

This got sent back to the Ranges Study Group as there are new design questions.

inbal2l commented 1 year ago

Reviewed by SG9 at Issaquah2023 meeting (Full Minutes).

Polls

POLL: Forward “P2406R3: Add ‘lazy_counted_iterator’” to LEWG for C++23.

SF F N A SA
2 3 3 5 1

Attendance: 31

# of Authors: 1

Author Position: SF

A: Would like to have it for C++26 A: Would like an alternative solution (which doesn’t requires the user to use a different utility)

Outcome: No consensus to move on for C++23 (NB comment will be rejected)

Author: As the paper states, it’s not easy to fix this in counted_iterator, there are parts that only the user knows about.

POLL: Forward “P2406R3: Add ‘lazy_counted_iterator’” to LEWG for C++26.

SF F N A SA
2 5 4 2 1

Attendance: 31

# of Authors: 1

Author Position: SF

Outcome: Weak consensus in favour

Summary

A few fixes for the paper were brought up during the meeting, the author will apply them and publish a new revision. SG9 was supportive of the design (and of adding a new utility), the main objection was to having this in C++23. (Chair note: R1 (which first proposed lazy_counted_iterator) was reviewed on November 8th, 2022)

brycelelbach commented 1 year ago

2023-02-07 08:30 to 10:15 Issaquah Library Evolution Meeting

P2406R5: Fix counted_iterator interaction with input iterators

2023-02-07 08:30 to 10:15 UTC-8 Issaquah Library Evolution Minutes

Champion: Yehezkel Bernat (R)

Chair: Steve Downey (IP) & Bryce Adelstein Lelbach (IP)

Minute Taker: Ben Craig (IP)

Start: 2023-02-07 09:25 UTC-8

POLL: Reject C++23 National Body comment US-46-107

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
3 8 9 2 2

Attendance: 17 (in person) + 18 (remote)

# of Authors: 1

Author Position: SA

Outcome: No consensus.

POLL: Forward P2406R4 to LWG for C++ 23

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
1 0 5 12 5

Attendance: 17 (in person) + 18 (remote)

# of Authors: 1

Author Position: SF

Outcome: Consensus against.

POLL: Change counted_iterator to have the proposed behavior of lazy_counted_iterator, which is a breaking change.

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
6 12 2 1 3

Attendance: 17 (in person) + 18 (remote)

# of Authors: 1

Author Position: ?

Outcome: Consensus in favor.

POLL: Change iterator category to at most forward

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

Attendance: 17 (in person) + 18 (remote)

# of Authors: 1

Author Position: ?

Outcome: No consensus, but leaning towards favoring this.

POLL: Require forward as minimum underlying iterator category

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
0 3 5 3 3

Attendance: 17 (in person) + 18 (remote)

# of Authors: 1

Author Position: ?

Outcome: No consensus.

POLL: Remove base from lazy_counted_iterator

Strongly Favor Weakly Favor Neutral Weakly Against Strongly Against
1 9 1 3 1

Attendance: 17 (in person) + 18 (remote)

# of Authors: 1

Author Position: SA

Outcome: Weak consensus in favor.

End: 10:30

Summary

This paper adds a new version of lazy_counting_iterator and take, to address some issues with counting_iterator that arise in take. During our discussion, there was debate about whether we should fix this in a new facility (as proposed), or make a breaking change to counting_iterator. We also discussed whether we should have base on lazy_counting_iterator, and if its behavior might be surprising. We considered whether we should require the underlying type to be forward iterator, and whether lazy_counting_iterator should ever model anything more than forward.

We did not have consensus to advance this paper as-is for C++23. We also did not have consensus to reject the C++23 National Body comment requesting this. We did have consensus that we'd like to do this as breaking change to counting_iterator instead of adding a new type.

Next Steps

The authors and volunteers will produce a revision of P2406 that allows call for either counted_iterator or have different behavior depending on the user code instead of adding a new facility. Then, Library Evolution will review it and attempt to build consensus for how to resolve US-46-107, the C++23 National Body comment requesting a fix for this.

brycelelbach commented 1 year ago

We're rejecting this National Body comment. The design of the proposed fix has evolved substantially over the last few days. We do not have consensus for change.

jensmaurer commented 1 year ago

Rejected. No consensus for a change.