Skip to content

Implement LWG-3785 ranges::to is over-constrained on the destination type being a range#3319

Merged
StephanTLavavej merged 2 commits intomicrosoft:mainfrom
frederick-vs-ja:lwg-3785
Jan 12, 2023
Merged

Implement LWG-3785 ranges::to is over-constrained on the destination type being a range#3319
StephanTLavavej merged 2 commits intomicrosoft:mainfrom
frederick-vs-ja:lwg-3785

Conversation

@frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jan 1, 2023

Fixes #3227.

I failed to figure out how to "convert a range of expected<T, E> to expected<vector<T>, E>" with ranges::to, so I just added some trivial test cases...

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner January 1, 2023 15:50
@CaseyCarter CaseyCarter added LWG Library Working Group issue ranges C++20/23 ranges labels Jan 2, 2023
CaseyCarter
CaseyCarter approved these changes Jan 2, 2023
View reviewed changes
stl/inc/ranges
template <class _Rng, class _Container>
concept _Ref_converts = convertible_to<range_reference_t<_Rng>, range_value_t<_Container>>;
concept _Ref_converts =
(!input_range<_Container>) || convertible_to<range_reference_t<_Rng>, range_value_t<_Container>>;
Copy link
Contributor

@CaseyCarter CaseyCarter Jan 2, 2023

Choose a reason for hiding this comment

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

For other reviewers: these parentheses are needed only to avoid clang-format damage.

@hewillk
Copy link
Contributor

hewillk commented Jan 8, 2023

I failed to figure out how to "convert a range of expected<T, E> to expected<vector<T>, E>" with ranges::to, so I just added some trivial test cases...

cc @brevzin

@brevzin
Copy link

brevzin commented Jan 8, 2023 •
edited
Loading

I failed to figure out how to "convert a range of expected<T, E> to expected<vector<T>, E>" with ranges::to, so I just added some trivial test cases...

cc @brevzin

Somebody would need to propose a new expected(from_range_t, R&&) constructor that does this, it's good that it doesn't work out of the box. The goal was simply to be able to support that kind of conversion for types that want to opt in to it.

And yeah, we should add that constructor (but... not in this PR of course).

@StephanTLavavej StephanTLavavej self-assigned this Jan 11, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 11, 2023

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit a71aa52 into microsoft:main Jan 12, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 12, 2023

Thanks for changing the STL to be more conformant and useful! 😹 🤪 🚀

@frederick-vs-ja frederick-vs-ja deleted the lwg-3785 branch January 12, 2023 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+2 more reviewers

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

LWG Library Working Group issue ranges C++20/23 ranges

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

LWG-3785 ranges::to is over-constrained on the destination type being a range

6 participants

@frederick-vs-ja @hewillk @brevzin @StephanTLavavej @CaseyCarter @strega-nil-ms