Skip to content

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

Merged
StephanTLavavej merged 2 commits into
microsoft:mainfrom
strega-nil:lwg-3701
Aug 3, 2022
Merged

Implement LWG-3701: Make formatter<remove_cvref_t<const charT[N]>, charT> requirement explicit#2957
StephanTLavavej merged 2 commits into
microsoft:mainfrom
strega-nil:lwg-3701

Conversation

@strega-nil-ms
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

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
Copy Markdown
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.

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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
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