Implement common_iterator by miscco · Pull Request #1092 · microsoft/STL

miscco · 2020-07-26T20:00:09Z

This implements common_iterator

I am at a point where I would like to get some feedback regarding certain issues

I have implemented it via a union rather than pulling in variant. unions scare me.
I had to additionally define the "classic" copy constructor/assignment because of the union. Do we also need move assignment?
I have never really worked with unions ( unions scare me) so I freely stole from optional/variant That is the reason I moved _Construct_in_place and _Nontrivial_dummy to xutility. Hiss if it does not belong there.
I am slightly concerned what happens if some of the preconditions are not met in release. unions scare me.
Similarly to counted_iterator i get strange errors about requires input_iterator<_Iter>
Tests. Urgh. operator->() is especially terrible thoughts?

tests/std/tests/P0896R4_common_iterator/test.cpp

stl/inc/xutility

miscco · 2020-07-27T10:12:03Z

@CaseyCarter the test currently fail because in the test machinery we require input_iterator for iter_swap.

So the question is is that requirement valid or should we expand it to indirectly_swappable

miscco · 2020-07-27T10:47:33Z

So I build this throwing_iterator to create valueless common_iterator. However it seems that I cannot use them as is in the death tests any exception directly terminates. Any suggestion on how to verify the asserts regarding the valueless_by_exception case?

miscco · 2020-07-28T10:22:31Z

I might be just spectacularly stupid. It seems I tried to swap elements of a const range and wondered why it didnt work...

At least locally all tests now work. Lets see what clang brings

miscco · 2020-07-28T17:17:32Z

I believe these test failures require maintainer support

miscco · 2020-07-29T07:14:57Z

So I was thinking a bit about exception safety and came up with the horror that is counted_iterator::operator=

Is that overkill?

stl/inc/optional

stl/inc/iterator

tests/std/tests/P0896R4_common_iterator/test.cpp

tests/std/tests/P0896R4_common_iterator_death/test.cpp

stl/inc/iterator

tests/std/tests/P0896R4_common_iterator_death/test.cpp

miscco · 2020-08-31T14:19:35Z

@CaseyCarter This should be ready for the next round of reviews

tests/std/tests/P0896R4_common_iterator/env.lst

tests/std/tests/P0896R4_common_iterator/test.cpp

stl/inc/iterator

tests/std/tests/P0896R4_common_iterator/test.cpp

tests/std/tests/P0896R4_common_iterator_death/test.cpp

CaseyCarter

Just some tweaky things which I'll fixup right now.

tests/std/tests/P0896R4_common_iterator_death/test.cpp

tests/std/tests/winsdk_concepts_matrix.lst

stl/inc/iterator

tests/std/tests/P0896R4_common_iterator/test.cpp

CaseyCarter · 2020-09-04T18:13:32Z

Thanks for implementing this decidedly un-common Ranges component! 😎

This reverts commit 30e7a1b.

…t#1265)" This reverts commit 6156ec3.

This reverts commit 6156ec3.

Implement common_iterator

759153f

miscco requested a review from a team as a code owner July 26, 2020 20:00

miscco commented Jul 26, 2020

View reviewed changes

tests/std/tests/P0896R4_common_iterator/test.cpp Outdated Show resolved Hide resolved

miscco added 2 commits July 26, 2020 22:05

Fix swap implementation of death test

e1fca3b

Add tests for iter_traits

24b54ac

miscco commented Jul 26, 2020

View reviewed changes

stl/inc/xutility Outdated Show resolved Hide resolved

StephanTLavavej added cxx20 C++20 feature ranges C++20/23 ranges labels Jul 26, 2020

miscco added 3 commits July 27, 2020 08:28

Fix swapped constructors

ea8bd71

Add indirection for common_iterator pointer type

b683fda

Some test fixes

d0787c2

miscco force-pushed the common_iterator branch from de0411b to d0787c2 Compare July 27, 2020 10:00

Fix death tests more

7246ac4

CaseyCarter mentioned this pull request Jul 27, 2020

P0896R4 <ranges> #39

Closed

miscco added 3 commits July 28, 2020 07:51

Fix assignment operator

967faa0

Writing is hard

5793a21

Swapping elements of a const range...

487123e

miscco added 2 commits July 28, 2020 16:32

typename

634fbde

Actually test the common_iterator

9d9a8ff

miscco force-pushed the common_iterator branch from 588bd2e to 9d9a8ff Compare July 28, 2020 15:00

miscco added 3 commits July 29, 2020 09:12

Ramp up exception safety

948b810

Merge branch 'master' into common_iterator

c01e72d

Guard _Nontrivial_dummy_type by C++17

27e7a8a

Fix typo

56a5f01

CaseyCarter self-requested a review July 29, 2020 20:12

miscco commented Aug 19, 2020

View reviewed changes

stl/inc/optional Outdated Show resolved Hide resolved

Remove unneeded C++17 guard

f245661

CaseyCarter suggested changes Aug 26, 2020

View reviewed changes

miscco added 4 commits August 28, 2020 13:20

Merge branch 'master' into common_iterator

c86f4ea

Address review comments and experiment with conversions

a41d984

Conversions actually work

91922ed

Create a tag type to test valueless common_iterator in death tests

1e1a890

miscco force-pushed the common_iterator branch from b41b17c to 1e1a890 Compare August 29, 2020 18:16

miscco added 2 commits August 31, 2020 14:19

Test operator->() for wrapped and proxy iterators

cd1eb5c

Add tests for the first codepathes of operator->

1a7e598

CaseyCarter self-requested a review September 2, 2020 21:31

CaseyCarter reviewed Sep 3, 2020

View reviewed changes

CaseyCarter suggested changes Sep 3, 2020

View reviewed changes

tests/std/tests/P0896R4_common_iterator_death/test.cpp Outdated Show resolved Hide resolved

tests/std/tests/P0896R4_common_iterator_death/test.cpp Outdated Show resolved Hide resolved

tests/std/tests/P0896R4_common_iterator_death/test.cpp Outdated Show resolved Hide resolved

Casey's review comments

e9d82a6

CaseyCarter approved these changes Sep 3, 2020

View reviewed changes

CaseyCarter removed their assignment Sep 3, 2020

StephanTLavavej requested changes Sep 3, 2020

View reviewed changes

STL's review comments

aa63981

CaseyCarter requested a review from StephanTLavavej September 4, 2020 05:26

StephanTLavavej approved these changes Sep 4, 2020

View reviewed changes

CaseyCarter self-assigned this Sep 4, 2020

CaseyCarter merged commit 30e7a1b into microsoft:master Sep 4, 2020

miscco deleted the common_iterator branch September 4, 2020 18:15

CaseyCarter removed their assignment Sep 4, 2020

CaseyCarter added a commit to CaseyCarter/STL that referenced this pull request Sep 4, 2020

Revert "Implement common_iterator (microsoft#1092)"

3a7a615

This reverts commit 30e7a1b.

CaseyCarter added a commit that referenced this pull request Sep 4, 2020

Revert "Implement common_iterator (#1092)" (#1265)

6156ec3

This reverts commit 30e7a1b.

CaseyCarter added a commit to CaseyCarter/STL that referenced this pull request Sep 5, 2020

Revert "Revert "Implement common_iterator (microsoft#1092)" (microsof…

6d52216

…t#1265)" This reverts commit 6156ec3.

CaseyCarter added a commit that referenced this pull request Sep 16, 2020

Revert "Revert "Implement common_iterator (#1092)" (#1265)" (#1268)

0e4f6b5

This reverts commit 6156ec3.

Conversation

miscco commented Jul 26, 2020

Uh oh!

Uh oh!

Uh oh!

miscco commented Jul 27, 2020

Uh oh!

miscco commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miscco commented Jul 28, 2020

Uh oh!

miscco commented Jul 28, 2020

Uh oh!

miscco commented Jul 29, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miscco commented Aug 31, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CaseyCarter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CaseyCarter commented Sep 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

miscco commented Jul 27, 2020 •

edited

Loading