Skip to content

Conversation

@cpplearner
Copy link
Contributor

This sentence comes from LWG 379, but it seems that there's a mistake when applying the proposed resolution to the draft standard: the proposed resolution says "For any named ctype category with a ctype<char> facet ctc", but the draft standard says "For any named ctype category with a ctype<charT> facet ctc".

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Seems a correct change, based on LWG379.

@jwakely ?

@cpplearner
Copy link
Contributor Author

ping @jwakely

@jwakely
Copy link
Member

jwakely commented Dec 13, 2024

Isn't the entire subject of LWG379 that the standard says charT and should say char? So if applying the issue failed to make that change, what did it do, just add a comma? Maybe the editor misread the note "[Kona: Minor edit. Added a comma after the M for clarity.]" as saying that was the required fix.

@jwakely
Copy link
Member

jwakely commented Dec 13, 2024

Ah it also changed ctw to ctc

Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

I've convinced my self that this change (as originally proposed by LWG379) is correct. It's strange that the original "typo" corrected by LWG379 also involved the name "ctw" which does strongly suggest that a ctype facet for Wide characters was intended, in contrast to do_narrow where it was always ctype<char> and ctc. If it was just a typo, you would not have expected the name "ctw" to also be a typo.

But since the argument to ctw.is(M, c) is a char it does make sense that ctype<charT> ctw was supposed to be ctype<char> ctc all along.

@tkoeppe tkoeppe merged commit c7fbd59 into cplusplus:main Dec 17, 2024
2 checks passed
@cpplearner cpplearner deleted the patch-1 branch December 18, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants