Skip to content

P1989R2 Range Constructor For string_view #2000

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Jun 29, 2021
Merged

P1989R2 Range Constructor For string_view #2000

merged 21 commits into from
Jun 29, 2021

Conversation

sam20908
Copy link
Contributor

@sam20908 sam20908 commented Jun 10, 2021 •
edited
Loading

Fixes #1977

  • Add tests
  • Add deduction guide

@StephanTLavavej StephanTLavavej added the cxx23 C++23 feature label Jun 10, 2021
@CaseyCarter CaseyCarter added the ranges C++20/23 ranges label Jun 10, 2021
@sam20908 sam20908 marked this pull request as ready for review June 11, 2021 01:35
@sam20908 sam20908 requested a review from a team as a code owner June 11, 2021 01:35
timsong-cpp
timsong-cpp reviewed Jun 11, 2021
View reviewed changes
@sam20908 @timsong-cpp
Address review comments
98e2d8a 
Co-authored-by: timsong-cpp <[email protected]>
CaseyCarter
CaseyCarter suggested changes Jun 11, 2021
View reviewed changes
@CaseyCarter CaseyCarter self-assigned this Jun 11, 2021
CaseyCarter
CaseyCarter approved these changes Jun 11, 2021
View reviewed changes
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Just one little nitpicky thing that I'll go ahead and apply.

@CaseyCarter
Update stl/inc/xstring
10de48d 
Remove extraneous `std`-qualification on `size_t`.
@CaseyCarter CaseyCarter removed their assignment Jun 11, 2021
@StephanTLavavej
Copy link
Member

This PR needs to merge main and resolve a conflict in yvals_core.h.

@sam20908
Copy link
Contributor Author

This PR needs to merge main and resolve a conflict in yvals_core.h.

On it.

@sam20908
Copy link
Contributor Author

Merge fixed.

@StephanTLavavej StephanTLavavej self-assigned this Jun 16, 2021
StephanTLavavej
StephanTLavavej reviewed Jun 17, 2021
View reviewed changes
@StephanTLavavej
Copy link
Member

@sam20908 @CaseyCarter After looking at the Standardese one more time, I added a bit more test coverage to exercise the various constraints. Some of them simply disambiguate other constructors so they didn't need additional testing. The one constraint that isn't exercised is sized_range but since it's in the product code I'm happy.

@StephanTLavavej

This comment has been minimized.

Sign in to view
@StephanTLavavej StephanTLavavej merged commit 6fe02ac into microsoft:main Jun 29, 2021
@StephanTLavavej
Copy link
Member

Thanks for implementing this C++23 feature - a major usability improvement that tons of programmers will enjoy! 🎉 😻 🚀

@sam20908 sam20908 deleted the cxx23_string_view_range_ctor branch June 30, 2021 13:46
@StephanTLavavej
Copy link
Member

Reported LLVM-51018 "C++23 makes constructing StringRef from SmallString ambiguous".

@statementreply statementreply mentioned this pull request Nov 24, 2021
Closed
@StephanTLavavej StephanTLavavej mentioned this pull request Dec 11, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timsong-cpp timsong-cpp timsong-cpp left review comments

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
cxx23 C++23 feature ranges C++20/23 ranges
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

P1989R2 Range Constructor For string_view
4 participants
@sam20908 @StephanTLavavej @CaseyCarter @timsong-cpp