Skip to content

Implement P2210R2 Superior String Splitting#2042

Merged
StephanTLavavej merged 29 commits into
microsoft:mainfrom
miscco:P2210-superior-string-splitting
Sep 2, 2021
Merged

Implement P2210R2 Superior String Splitting#2042
StephanTLavavej merged 29 commits into
microsoft:mainfrom
miscco:P2210-superior-string-splitting

Conversation

@miscco
Copy link
Copy Markdown
Contributor

@miscco miscco commented Jul 6, 2021

This implements the first part of P2210R2 renaming split_view to lazy_split_view

Works towards #1980.

@miscco miscco requested a review from a team as a code owner July 6, 2021 16:02
@miscco miscco force-pushed the P2210-superior-string-splitting branch from 57ef4a1 to f0ac30c Compare July 6, 2021 19:22
@StephanTLavavej StephanTLavavej added cxx20 C++20 feature defect report Applied retroactively ranges C++20/23 ranges labels Jul 6, 2021
Comment thread stl/inc/ranges Outdated
@miscco miscco force-pushed the P2210-superior-string-splitting branch from f0ac30c to 1fae222 Compare July 7, 2021 06:25
@miscco miscco changed the title Implement first part of P2210R2 Superior String Splitting Implement P2210R2 Superior String Splitting Jul 19, 2021
Comment thread tests/std/tests/P0896R4_ranges_range_machinery/test.cpp
Comment thread tests/std/tests/P0896R4_views_split/test.cpp Outdated
Co-authored-by: Adam Bucior <[email protected]>
Comment thread stl/inc/ranges
Comment thread stl/inc/ranges
Comment thread stl/inc/ranges
Comment thread stl/inc/ranges
@miscco miscco force-pushed the P2210-superior-string-splitting branch from d3101f4 to 7e9e444 Compare July 20, 2021 16:35
@StephanTLavavej
Copy link
Copy Markdown
Member

@miscco - Are you still working on resolving @timsong-cpp's comments, or were they resolved in the recent commits? Just wondering whether this should move to Initial Review again. 😺

@miscco
Copy link
Copy Markdown
Contributor Author

miscco commented Jul 22, 2021

@miscco - Are you still working on resolving @timsong-cpp's comments, or were they resolved in the recent commits? Just wondering whether this should move to Initial Review again. 😺

Actually this is something that I would want to get @CaseyCarter's opinion on. As far as I remember his analysis was that we do not need to do anything related to P2281 and if we change something here I would want to be sure we change it consistently everywhere

Comment thread stl/inc/xutility Outdated
Comment thread stl/inc/xutility Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread tests/std/tests/P0896R4_views_lazy_split/test.cpp Outdated
Comment thread stl/inc/ranges
Comment thread tests/std/tests/P0896R4_views_split/test.cpp Outdated
@CaseyCarter CaseyCarter linked an issue Aug 19, 2021 that may be closed by this pull request
Comment thread stl/inc/ranges
Comment thread stl/inc/ranges
Comment thread tests/std/tests/P0896R4_views_split/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_views_split/test.cpp Outdated
@miscco
Copy link
Copy Markdown
Contributor Author

miscco commented Aug 25, 2021

@CaseyCarter I would also suggest to move this to a follow up. As far as I can tell there are two things we need to do

  1. Improve the perfect forwarding for the adaptors
  2. Iplement decay-copy for split_view and lazy_split_view

Given that those are non trivial changes (and I currently do not have that much capacity for delicate tasks) I feel it is better to extract them into a follow up.

@StephanTLavavej

This comment has been minimized.

@CaseyCarter CaseyCarter removed their assignment Aug 31, 2021
@StephanTLavavej StephanTLavavej self-assigned this Aug 31, 2021
Copy link
Copy Markdown
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! I'll push changes to address my feedback (they were all localized).

Comment thread stl/inc/xutility Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread tests/std/tests/P0896R4_views_split/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_views_split/test.cpp Outdated
Comment thread tests/std/tests/P0896R4_views_lazy_split/test.cpp
@StephanTLavavej StephanTLavavej removed their assignment Sep 1, 2021
@miscco
Copy link
Copy Markdown
Contributor Author

miscco commented Sep 1, 2021

Thanks a lot for the review and the fixes 😸

@StephanTLavavej StephanTLavavej self-assigned this Sep 2, 2021
@StephanTLavavej
Copy link
Copy Markdown
Member

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

@StephanTLavavej StephanTLavavej merged commit 21961bb into microsoft:main Sep 2, 2021
@StephanTLavavej
Copy link
Copy Markdown
Member

"Thanks", "For", "Implementing", "This", "C++20", "Defect", "Report" 😸

@miscco miscco deleted the P2210-superior-string-splitting branch September 2, 2021 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cxx20 C++20 feature defect report Applied retroactively ranges C++20/23 ranges

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P2210R2 Superior String Splitting

5 participants