Skip to content

Speculatively implement LWG-3857 #3348

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
StephanTLavavej merged 7 commits into from
Jan 22, 2023
Merged

Speculatively implement LWG-3857 #3348

StephanTLavavej merged 7 commits into from
Jan 22, 2023

Conversation

@frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jan 16, 2023 •
edited
Loading

Fixes #3336.

The involved constructor of basic_string_view is introduced in C++23, so this PR also fixes the issue in C++20 mode in an alternative way.

Also updates references to working darft in <format> to WG21-N4928.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner January 16, 2023 01:46
@StephanTLavavej StephanTLavavej added LWG Library Working Group issue format C++20/23 format labels Jan 18, 2023
cpplearner
cpplearner reviewed Jan 18, 2023
View reviewed changes
frederick-vs-ja and others added 2 commits January 18, 2023 17:20
@frederick-vs-ja @cpplearner
Address @cpplearner's review comments
760e925 
And update references to WG21-N4928.

Co-authored-by: S. B. Tam <cpplearner@outlook.com>
@frederick-vs-ja
Test coverage for non-Standard traits_type
a86e821
strega-nil-ms
strega-nil-ms approved these changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

I do also appreciate the C++20 workaround. I personally think it's the best way forward.

@StephanTLavavej StephanTLavavej self-assigned this Jan 18, 2023
StephanTLavavej
StephanTLavavej reviewed Jan 19, 2023
View reviewed changes
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 19, 2023

Looks great, thanks! I pushed a trivial commit to unqualify std::allocator in the test.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 21, 2023

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

@StephanTLavavej StephanTLavavej merged commit 4f5d13a into microsoft:main Jan 22, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 22, 2023

Thanks for improving basic_string_view's usability! 😸 🧵 🧶

@frederick-vs-ja frederick-vs-ja deleted the lwg-3857 branch January 22, 2023 06:57
@StephanTLavavej StephanTLavavej mentioned this pull request Jan 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+2 more reviewers

@cpplearner cpplearner cpplearner left review comments

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

<format>: Failure to format basic_string_view with non-standard traits

4 participants

@frederick-vs-ja @StephanTLavavej @cpplearner @strega-nil-ms