Skip to content

Implement resolution of LWG-3449 for take_view - #1457

Merged
Stephan T. Lavavej (StephanTLavavej) merged 4 commits into
microsoft:masterfrom
miscco:lwg3449
Dec 17, 2020
Merged

Implement resolution of LWG-3449 for take_view#1457
Stephan T. Lavavej (StephanTLavavej) merged 4 commits into
microsoft:masterfrom
miscco:lwg3449

Conversation

@miscco

@miscco Michael Schellenberger Costa (miscco) commented Nov 12, 2020

Copy link
Copy Markdown
Contributor

Addresses #1446

Fixes #1463.

Comment thread stl/inc/ranges Outdated
Comment thread tests/std/tests/P0896R4_views_take/test.cpp Outdated
* Correct the requirement on the new `operator==` overload
* Rename `_Base_Ty` to `_Base_t` and `_Counted_Iter` to `_Counted_iter` to be consistent with our naming conventions (snakes and camels don't mix).
Comment thread stl/inc/ranges Outdated
@CaseyCarter Casey Carter (CaseyCarter) removed their assignment Dec 3, 2020
@CaseyCarter

Copy link
Copy Markdown
Contributor

Michael Schellenberger Costa (@miscco) Stephan T. Lavavej (@StephanTLavavej) FYI I pushed a commit to change the names _Counted_Iter and _Base_Ty to be consistent with our naming conventions (snakes and camels don't mix) and to correct the constraint on the new operator== overload.

@StephanTLavavej

Copy link
Copy Markdown
Member

Michael Schellenberger Costa (@miscco) Casey Carter (@CaseyCarter) Does this indicate that we're missing operator== test coverage?

@CaseyCarter

Copy link
Copy Markdown
Contributor

Michael Schellenberger Costa (@miscco) Casey Carter (@CaseyCarter) Does this indicate that we're missing operator== test coverage?

Yes: the take_view test passes with the new operator== commented out. We need to investigate. (Maybe our test ranges all model simple-view?)

@CaseyCarter

Copy link
Copy Markdown
Contributor

Maybe our test ranges all model simple-view?

Why yes, yes they do.

@miscco

Copy link
Copy Markdown
Contributor Author

I would suggest that we merge this and then fix the simple_view issue globally.

I believe there will be sufficient breakage coming from it for many PRs

@CaseyCarter

Copy link
Copy Markdown
Contributor

I would suggest that we merge this and then fix the simple_view issue globally.

I believe there will be sufficient breakage coming from it for many PRs

I agree; I've updated #1409 to include addressing this problem in all of the view tests. I'll move this to Final Review and we'll see if Stephan T. Lavavej (@StephanTLavavej) agrees with that strategy.

@StephanTLavavej

Copy link
Copy Markdown
Member

That strategy sounds good to me, thanks. I've pushed a merge to test this with 16.9 Preview 2 (no conflicts).

@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for implementing this LWG issue resolution and increasing the crystalline perfection of ranges! 💎 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LWG-3449 take_view and take_while_view's sentinel<false> not comparable with their const iterator

3 participants