Skip to content

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

Merged
Stephan T. Lavavej (StephanTLavavej) merged 2 commits into
microsoft:mainfrom
strega-nil:lwg-3701
Aug 3, 2022
Merged

Stephan T. Lavavej (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
nicole mazzuca (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 Casey Carter (CaseyCarter) added LWG Library Working Group issue format C++20/23 format labels Jul 27, 2022

@CaseyCarter Casey Carter (CaseyCarter) left a comment •

Copy link
Copy Markdown
Contributor

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.

@frederick-vs-ja A. Jiang (frederick-vs-ja) left a comment •

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Member

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

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 12c5143 into microsoft:main Aug 3, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for implementing another LWG issue resolution! 📉 😻 ✅

nicole mazzuca (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>
Igor Zhukov (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