-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Character Literals (#1934) #1964
Character Literals (#1934) #1964
Conversation
70f85aa
to
8fac93f
Compare
8fac93f
to
22a5c69
Compare
87b433e
to
2660f2f
Compare
@opelolo let me know when you want to start collaborating on this |
5b22640
to
630bfaa
Compare
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
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.
Sorry for the delay getting back to you on this. GSoC and so on have been keeping me busy.
Looks good! I think this is ready for the leads to review.
@chandlerc could you take another look at this when you have a chance? |
@zygoloid could you take a look at this when you have a chance? |
FYI both @zygoloid and @chandlerc have been on vacation (I think Chandler still is), so it may take them a little while to get to this. |
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.
Looks good, I'm happy to move forward with this.
- Only one operand may be a character literal, the other must be an | ||
integer literal. |
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.
One consequence appears to be that this is invalid:
fn Digit(n: i8) -> Char32 {
return '0' + n;
}
... and something like return ('0' as Char32) + n;
would be needed instead. I think I'm OK with that, but I expect it to be a minor source of friction as that kind of usage is fairly common in C++.
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.
Given that '0' + n
has a value that is only known at runtime, what type should it be? Using the type of n
here is a bit worrisome, due to overflow. I would be fine with saying the result would be Char32
, but maybe that would only make sense for some types of n
? For example if n: i64
, a Char32
result would be surprising.
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 think I like keeping this super-explicit for now (requiring a cast to a specific sized type). We can try to add better defaults if in practice this friction is something users dislike. I'm somewhat hopeful that instead we can have a really easily discovered (and targeted in migration) API for mapping to digits, and avoid how much this comes up in practice. But it seems easy to address if it does come up.
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
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.
First off, sorry for all the delays while folks were first at a conference, and then on vacation, and then catching up! I know that really stretched things out way more than we'd like for this proposal, and I appreciate you bearing with us.
Hopefully things will move (much) more steadily now.
On this proposal, I'm pretty happy moving forward here too. =] Thanks so much for the work on this!
All my comments below are just around wording, clarifications, etc. At least, I don't think there are any suggestions for a substantive change here. Hopefully with help from folks we can converge quickly on the remaining wording issues and land this.
- Only one operand may be a character literal, the other must be an | ||
integer literal. |
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 think I like keeping this super-explicit for now (requiring a cast to a specific sized type). We can try to add better defaults if in practice this friction is something users dislike. I'm somewhat hopeful that instead we can have a really easily discovered (and targeted in migration) API for mapping to digits, and avoid how much this comes up in practice. But it seems easy to address if it does come up.
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
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.
Looks great, thanks so much! Merging!
Thank you everyone! |
Put character literals in single quotes, like
'a'
. Character literals worklike numeric literals:
assigned to, not the literal itself. Follows the plan from Open design idea: character literals #1934.