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.

Sign up for GitHub

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

Jump to bottom

Implement LWG-3798 Rvalue reference and iterator_category #3359

Merged
merged 6 commits into from
Jan 28, 2023
Merged

Implement LWG-3798 Rvalue reference and iterator_category #3359

merged 6 commits into from
Jan 28, 2023

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jan 24, 2023 •
edited by StephanTLavavej
Loading

Fixes #3228.

Also updates references to working draft in <ranges> and P0896R4_ranges_iterator_machinery/test.cpp to WG21-N4928.

Not all changes in LWG-3798 are implemented in this PR.

@frederick-vs-ja
Implement LWG-3798
9f6430d 
For:
- `cpp17-forward-iterator`
- `transform_view`
- `join_with_view`
@frederick-vs-ja
Test coverage for LWG-3798
2f9ba47 
For `cpp17-forward-iterator`, `transform_view`, and `join_with_view`.
@frederick-vs-ja
Update references to WD to WG21-N4928
92b95bc
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner January 24, 2023 15:45
@strega-nil-ms

This comment was marked as resolved.

Sign in to view
@StephanTLavavej StephanTLavavej added the LWG Library Working Group issue label Jan 24, 2023
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@frederick-vs-ja
Copy link
Contributor Author

I analyzed the buggy cases for Clang and reported LLVM-60293.

There are a large number of default constructors that are defaulted and constrained by default_initializable in [ranges]. I'm not sure whether workaround should be introduced into product code.

@StephanTLavavej StephanTLavavej self-assigned this Jan 25, 2023
strega-nil-ms
strega-nil-ms approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

StephanTLavavej
StephanTLavavej reviewed Jan 26, 2023
View reviewed changes
stl/inc/ranges Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_views_transform/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jan 26, 2023
@StephanTLavavej
Copy link
Member

Thanks for implementing this, adding significant test coverage, and investigating/reporting the Clang bug! Looks good to me too, after pushing minor changes to fix WP citations, a pre-existing typo, const chirality, and some unused using-declarations. FYI @strega-nil-ms.

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

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

@StephanTLavavej StephanTLavavej merged commit 7ab764d into microsoft:main Jan 28, 2023
@StephanTLavavej
Copy link
Member

Thanks for implementing and extensively testing this LWG issue resolution! 📉 😻 💯

@frederick-vs-ja frederick-vs-ja deleted the lwg-3798 branch January 28, 2023 03:36
@frederick-vs-ja frederick-vs-ja mentioned this pull request Feb 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
LWG Library Working Group issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LWG-3798 Rvalue reference and iterator_category
3 participants
@frederick-vs-ja @strega-nil-ms @StephanTLavavej