-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add support for char8_t (from C++20). #26153
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
Closed
varungandhi-apple
wants to merge
1
commit into
swiftlang:master
from
varungandhi-apple:vg-add-char8_t-support
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@airspeedswift, how are we handling availability for new top-level typealiases? These can't affect ABI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I suggested Varun pick this up as a starter-ish task, but this is a public change to the stdlib, and
CChar8
is close toCChar
. Does it need formal review or even a proposal?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, given that WG14 is working on a TN for
char16_t
andchar32_t
, I could seechar8_t
being introduced into C, where it gets interesting with-fsigned-char
and-fno-signed-char
. I think that on Darwin targets where you default to-fsigned-char
, havingCChar
besigned char
, andCChar8
beunsigned char
is slightly scary. Might be worth bringing this up through a proposal.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@compnerd but what's the alternative, really? Yeah, it would be slightly messy, but there's really no other option on the table except maybe "just don't import these types". I think this is a thing that we just live with.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced that "just don't import these types" is a viable solution (particularly long term). Sure, we can get away with it for the time being, but, I don't think that we would be able to escape it forever.
Now, I'm not saying that this is what we should do or even that what I'm suggesting is good. Hopefully someone can come up with something more clever.
We could import this as
CXX.Char8
(whereCXX
is a@frozen
enum which I believe should allow inling).Yes, I am a bad person doing bad things to the compiler :-(.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, non-generic containing contexts don't really affect codegen at all for nested types. So yeah, that is technically an option. It's a weird choice if C22 adopts
char8_t
as well, though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we could introduce
CChar8
at that time for the Cchar8_t
and if they make it compatible withchar
(including-fsigned-char
), we can handle both.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see
char8_t
ever being signed; it's defined as a UTF-8 code unit. And we can't introduceCChar8
separate fromCXX.Char8
because they'd be different types you could overload on (unless one is a typealias).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, they're all typealiases. Retracted, at least partially.