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

[2025-02 LWG 14] P2846R6 reserve_hint: Eagerly reserving memory for not-quite-sized lazy ranges #7698

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

burblebee
Copy link
Contributor

Fixes #7673.

@burblebee burblebee marked this pull request as ready for review February 19, 2025 08:15
@Eisenwave
Copy link
Contributor

Fixes cplusplus/papers#1549.

Comment on lines +1168 to +1169
%FIXME: What is "an approximately_sized_range"?
an \libconceptx{approximate\-ly_sized_range}{approximately_sized_range} whose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is trying to say

[...] to be well-defined for a type modeling approximately_sized_range

This area needs major fixups anyway because there is an example that is not \begin{example} but just kinda sits there in the middle of a note. I also feel like this wording is super clunky, so here's the idea:

[ Example: If t is of type U whose iterator type does not model forward_iterator, and if after a call to ranges::begin(t), ranges::reserve_hint(t) is no longer well-defined,
U still models approximately_sized_range if the other requirements are satisfied. — end example ]

I don't feel like this example is adding any value one way or the other though, and we could just delete it. No matter how you phrase it, it just seems to reiterate the other wording in the note.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this phase, we're focusing on making sure the pull request represents the voted-on paper as faithfully as possible. So, if you find deviations between the pull request and the wording in the incoming paper, those need to be addressed as "fixup"s.

Any other changes must be separate commits, and must be carefully weighed whether they are editorial or not. Removing a note is certainly editorial on the face of it, but @jwakely should have an opinion.

Copy link
Contributor

@Eisenwave Eisenwave Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do note that the editor left a %FIXME here, and there's something clearly defective in the paper wording here. And yeah, fixup commits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this sentence is modeled (!) on existing wording https://eel.is/c++draft/range.sized#note-1 - so any perceived defect here exists elsewhere already.

\tcode{T} models \libconcept{approximately_sized_range} only if
\begin{itemize}
\item
\tcode{ranges::reserve_hint(t)} is amortized \bigoh{1},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
\tcode{ranges::reserve_hint(t)} is amortized \bigoh{1},
\tcode{ranges::reserve_hint(t)} has amortized constant complexity,

It doesn't seem correct to say that [C++ expression] is [complexity]. That is totally colloquial.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have one other appearance of that phrasing, when defining sized_range. So, it seems to be locally consistent.

@cor3ntin
Copy link
Contributor

It would seem the changes adding index entries are unrelated and should be done in a separate (editorial) PR

@burblebee
Copy link
Contributor Author

burblebee commented Feb 22, 2025

It would seem the changes adding index entries are unrelated and should be done in a separate (editorial) PR

I believe indexes are editorial and discretionary. We add them as we see fit. That said, I did go a bit overboard here; I probably should have just fixed the references added in this paper, but since it's purely editorial (they don't change any wording), I'm hoping it's OK.

\pnum
\recommended
If \tcode{R} models \tcode{ranges::\libconcept{approximately_sized_range}} and
\tcode{ranges::distance(\linebreak{}rg) <= ranges::reserve_hint(rg)} is \tcode{true},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the linebreak here? If we need a break, after and should do it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linebreak is to prevent "Overfull"ness. To place the linebreak after the "and" would cause Letex to stretch the line out to the right margin, which would be ugly. What I've done is most consistent with how we've handled similar cases elsewhere.

\pnum
\recommended
If \tcode{R} models \tcode{ranges::\libconcept{approximately_sized_range}} and
\tcode{ranges::distance(\linebreak{}rg) <= ranges::reserve_hint(rg)} is \tcode{true},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2025-02 LWG Motion 14] P2846R6 reserve_hint: Eagerly reserving memory for not-quite-sized lazy ranges
4 participants