Skip to content

Implement LWG-3798 Rvalue reference and iterator_category - #3359

Merged
Stephan T. Lavavej (StephanTLavavej) merged 6 commits into
microsoft:mainfrom
frederick-vs-ja:lwg-3798
Jan 28, 2023
Merged

Implement LWG-3798 Rvalue reference and iterator_category#3359
Stephan T. Lavavej (StephanTLavavej) merged 6 commits into
microsoft:mainfrom
frederick-vs-ja:lwg-3798

Conversation

@frederick-vs-ja

@frederick-vs-ja A. Jiang (frederick-vs-ja) commented Jan 24, 2023 •
edited by StephanTLavavej
Loading

Copy link
Copy Markdown
Contributor

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
A. Jiang (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 Stephan T. Lavavej (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

A. Jiang (frederick-vs-ja) commented Jan 25, 2023

Copy link
Copy Markdown
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.

strega-nil-ms
nicole mazzuca (strega-nil-ms) approved these changes Jan 25, 2023
View reviewed changes

@strega-nil-ms nicole mazzuca (strega-nil-ms) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Thanks!

StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) reviewed Jan 26, 2023
View reviewed changes
Comment thread stl/inc/ranges Outdated
Comment thread tests/std/tests/P0896R4_views_transform/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp Outdated
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jan 26, 2023

Copy link
Copy Markdown
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 nicole mazzuca (@strega-nil-ms).

@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jan 27, 2023

Copy link
Copy Markdown
Member

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

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 7ab764d into microsoft:main Jan 28, 2023
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jan 28, 2023

Copy link
Copy Markdown
Member

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

@frederick-vs-ja A. Jiang (frederick-vs-ja) mentioned this pull request Feb 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) StephanTLavavej approved these changes
+1 more reviewer
@strega-nil-ms nicole mazzuca (strega-nil-ms) strega-nil-ms approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej Stephan T. Lavavej (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