Skip to content

[alg.ends.with]: drop_view should be views::drop #6773

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

Merged
merged 1 commit into from
May 20, 2024

Conversation

Quuxplusone
Copy link
Contributor

We don't want programmers doing CTAD on ranges::drop_view(args...), so we shouldn't put it in the as-if-by code either. What we expect people to do (and what we intend vendors to do here) is views::drop.

(Compare #6373. I'm also pinging the LWG reflector for an opinion on whether this is handleable without an LWG issue.)

@Quuxplusone
Copy link
Contributor Author

Quuxplusone commented Jan 16, 2024

Updated (thanks Tim). The ranges::ref_view(r1) wrapper is necessary for owning_view: https://godbolt.org/z/8secz55Tn
Here I'd like to use views::ref(r1) but that doesn't exist; or views::all(r1) but that's ill-formed for owning_views; I don't think there's a user-facing way to spell "I just want to refer to this view, not copy or modify it."

EDIT: ...oh, unless the user-facing spelling is simply std::move(r1)! We are trashing the view (by iterating it), so it's not like we need to preserve its old value. But moving an owning_view is likely a runtime pessimization, compared to just taking a ref_view to it.

@timsong-cpp
Copy link
Contributor

We are trashing the view (by iterating it)

Move-only views are not necessarily input-only.

We don't want programmers doing CTAD on `ranges::drop_view(args...)`,
so we shouldn't put it in the as-if-by code either. What we expect
people to do (and what we intend vendors to do here) is `views::drop`.

Tim Song points out that when `r1` is a move-only view such as an
`owning_view`, then `views::drop(r1, N)` alone is ill-formed because
`views::all(r1)` is ill-formed. We have to manually wrap the argument
`r1` in a `ranges::ref_view` (for which there is no user-facing name)
in order to handle `owning_view` correctly.
@Quuxplusone
Copy link
Contributor Author

@tkoeppe @jwakely ping!

@jensmaurer jensmaurer merged commit 2f23560 into cplusplus:main May 20, 2024
jensmaurer pushed a commit to jensmaurer/draft that referenced this pull request Jul 28, 2024
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.

4 participants