Skip to content

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

Merged
Stephan T. Lavavej (StephanTLavavej) merged 7 commits into
microsoft:mainfrom
gogim1:LWG3710
Jul 28, 2022
Merged

Implement LWG-3710: The end of chunk_view for input ranges can be const#2878
Stephan T. Lavavej (StephanTLavavej) merged 7 commits into
microsoft:mainfrom
gogim1:LWG3710

Conversation

@gogim1

@gogim1 Jinze Wu (gogim1) commented Jul 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Implements the proposed resolution of LWG-3710

@gogim1
fix LWG-3710
4a72974 
Signed-off-by: gogim1 <294843472@qq.com>
@gogim1
Jinze Wu (gogim1) requested a review from a team as a code owner July 19, 2022 02:45
@gogim1
Jinze Wu (gogim1) marked this pull request as draft July 19, 2022 03:01
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added the LWG Library Working Group issue label Jul 19, 2022
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jul 19, 2022

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

A. Jiang (frederick-vs-ja) commented Jul 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think this test file should also be changed.

STL/tests/std/tests/P2442R1_views_chunk/test.cpp

Line 200 in ef62d3f

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:

STL/tests/std/tests/P2442R1_views_chunk/test.cpp

Line 205 in ef62d3f

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

And forward_range should be detected in this line.

@frederick-vs-ja A. Jiang (frederick-vs-ja) mentioned this pull request Jul 19, 2022
Closed
26 tasks
@gogim1
fix static_assert failed
981c02e 
Signed-off-by: gogim1 <294843472@qq.com>
@ghost

Deleted user (ghost) commented Jul 19, 2022 •
edited by ghost
Loading

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@gogim1
fix code format
3df7fa4
@gogim1
fix code format
c9beaaf 
Signed-off-by: gogim1 <294843472@qq.com>
gogim1
Jinze Wu (gogim1) commented Jul 19, 2022
View reviewed changes
Comment thread tests/std/tests/P2442R1_views_chunk/test.cpp Outdated
miscco
Michael Schellenberger Costa (miscco) reviewed Jul 19, 2022
View reviewed changes
Comment thread tests/std/tests/P2442R1_views_chunk/test.cpp Outdated
@CaseyCarter Casey Carter (CaseyCarter) added the blocked Something is preventing work on this label Jul 19, 2022
@CaseyCarter

Casey Carter (CaseyCarter) commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

This looks good, other than the formatting issue.

@gogim1
force clang-formats hand by adding a comment
2b86225 
Signed-off-by: gogim1 <294843472@qq.com>
gogim1
Jinze Wu (gogim1) commented Jul 20, 2022
View reviewed changes
Comment thread tests/std/tests/P2442R1_views_chunk/test.cpp Outdated
@CaseyCarter Casey Carter (CaseyCarter) removed their assignment Jul 21, 2022
CaseyCarter
Casey Carter (CaseyCarter) reviewed Jul 21, 2022
View reviewed changes
Comment thread tests/std/tests/P2442R1_views_chunk/test.cpp Outdated
@CaseyCarter
Casey Carter (CaseyCarter) marked this pull request as ready for review July 21, 2022 00:25
@gogim1

Jinze Wu (gogim1) commented Jul 21, 2022

Copy link
Copy Markdown
Contributor Author

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

@CaseyCarter Casey Carter (CaseyCarter) removed the blocked Something is preventing work on this label Jul 25, 2022
StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) reviewed Jul 26, 2022
View reviewed changes

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) left a comment

Copy link
Copy Markdown
Member

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.

Comment thread tests/std/tests/P2442R1_views_chunk/test.cpp Outdated
Comment thread tests/std/tests/P2442R1_views_chunk/test.cpp Outdated
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jul 26, 2022

Copy link
Copy Markdown
Member

FYI Casey Carter (@CaseyCarter), I pushed very minor changes after you approved.

@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jul 27, 2022

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 b4d14a6 into microsoft:main Jul 28, 2022
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Jul 28, 2022

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

Igor Zhukov (fsb4000) pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
@gogim1 @CaseyCarter
@StephanTLavavej @fsb4000
Implement LWG-3710: The end of chunk_view for input ranges can be…
89cc6a2 
… `const` (microsoft#2878)

Co-authored-by: Casey Carter <cacarter@microsoft.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
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
+3 more reviewers
@CaseyCarter Casey Carter (CaseyCarter) CaseyCarter approved these changes
@miscco Michael Schellenberger Costa (miscco) miscco approved these changes
@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.

6 participants

@gogim1 @StephanTLavavej @frederick-vs-ja @CaseyCarter @miscco @strega-nil-ms