Skip to content
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.

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

Implement LWG-3701: Make formatter<remove_cvref_t<const charT[N]>, charT> requirement explicit #2957

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

strega-nil-ms
Copy link
Contributor

Fixes #2953

Make `formatter<remove_cvref_t<const charT[N]>, charT>`
requirement explicit
@strega-nil-ms strega-nil-ms requested a review from a team as a code owner July 27, 2022 17:49
@strega-nil-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@CaseyCarter CaseyCarter added LWG Library Working Group issue format C++20/23 format labels Jul 27, 2022
Copy link
Member

@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.

Nitpicky change. I think it's reasonable that we don't have new test coverage. Since we're enforcing a better requirement than what the standard specifies, the problem cited in the issue isn't a problem for us (https://godbolt.org/z/dsoMseMnh).

EDIT: Oops - I apparently didn't commit the actual comment. For posterity (Nicole asked me in person) I asked for formatter<T[N], ...> and formatter<const T[N], ...> to derive from the same specialization of _Formatter_base since const on the first type parameter doesn't affect _Formatter_base.

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

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

No change requested. I guess we can test this with the (duplicate of) std::formattable concept added by P2286R8. But since this will be tested alongwith P2286R8, it may be not worthy to duplicate such test.

@StephanTLavavej StephanTLavavej self-assigned this Aug 3, 2022
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 12c5143 into microsoft:main Aug 3, 2022
@StephanTLavavej
Copy link
Member

Thanks for implementing another LWG issue resolution! 📉 😻 ✅

strega-nil added a commit to strega-nil/stl that referenced this pull request Aug 6, 2022
…harT>` requirement explicit (microsoft#2957)

Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com>
@strega-nil strega-nil deleted the lwg-3701 branch August 10, 2022 20:35
fsb4000 pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
…harT>` requirement explicit (microsoft#2957)

Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format C++20/23 format LWG Library Working Group issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LWG-3701 Make formatter<remove_cvref_t<const charT[N]>, charT> requirement explicit
6 participants