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

Implement LWG-3710: The end of chunk_view for input ranges can be const #2878

Merged
merged 7 commits into from
Jul 28, 2022

Conversation

gogim1
Copy link
Contributor

@gogim1 gogim1 commented Jul 19, 2022

Implements the proposed resolution of LWG-3710

Signed-off-by: gogim1 <[email protected]>
@gogim1 gogim1 requested a review from a team as a code owner July 19, 2022 02:45
@gogim1 gogim1 marked this pull request as draft July 19, 2022 03:01
@StephanTLavavej StephanTLavavej added the LWG Library Working Group issue label Jul 19, 2022
@StephanTLavavej
Copy link
Member

Welcome to the microsoft/STL repo!

The P2442R1_views_chunk test is failing with:

C:\a\1\s\tests\std\tests\P2442R1_views_chunk\test.cpp(200): error C2338: static_assert failed: 'CanMemberEnd<const R> == forward_range<const V>'
C:\a\1\s\tests\std\tests\P2442R1_views_chunk\test.cpp(204): error C2338: static_assert failed: 'common_range<R> == (common_range<const V> && (sized_range<const V> || !bidirectional_range<const V>) )'
C:\a\1\s\tests\std\tests\P2442R1_views_chunk\test.cpp(202): error C7601: the associated constraints are not satisfied

Please ensure that this test passes locally before pushing changes. Let us know if you need assistance!

@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Jul 19, 2022

I think this test file should also be changed.

STATIC_ASSERT(CanMemberEnd<const R> == forward_range<const V>);

Due to LWG-3710, CanMemberEnd<const R> becomes true if const V is an input_range.


Edit:

common_range<R> == (common_range<const V> && (sized_range<const V> || !bidirectional_range<const V>) ));

And forward_range should be detected in this line.

@ghost
Copy link

ghost commented Jul 19, 2022

CLA assistant check
All CLA requirements met.

@CaseyCarter CaseyCarter added the blocked Something is preventing work on this label Jul 19, 2022
@CaseyCarter
Copy link
Member

This looks good, other than the formatting issue.

@CaseyCarter CaseyCarter removed their assignment Jul 21, 2022
@CaseyCarter CaseyCarter marked this pull request as ready for review July 21, 2022 00:25
@gogim1
Copy link
Contributor Author

gogim1 commented Jul 21, 2022

Thanks everyone. Without your help, I could not have finished this patch.

@CaseyCarter CaseyCarter removed the blocked Something is preventing work on this label Jul 25, 2022
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great! I'll push minor changes for nitpicky style issues.

tests/std/tests/P2442R1_views_chunk/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P2442R1_views_chunk/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

FYI @CaseyCarter, I pushed very minor changes after you approved.

@StephanTLavavej StephanTLavavej self-assigned this Jul 27, 2022
@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 b4d14a6 into microsoft:main Jul 28, 2022
@StephanTLavavej
Copy link
Member

Thanks for improving <ranges> conformance, and congratulations on your first microsoft/STL commit! 😻 🎉 🚀

This change will be available in VS 2022 17.4 Preview 2.

fsb4000 pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
… `const` (microsoft#2878)

Co-authored-by: Casey Carter <[email protected]>
Co-authored-by: Stephan T. Lavavej <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LWG Library Working Group issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants