-
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
Polish out support for reals and strings. #2593
Conversation
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.
LG with the below comment addressed.
@@ -156,6 +156,7 @@ CARBON_KEYWORD_TOKEN(Return, "return") | |||
CARBON_KEYWORD_TOKEN(Returned, "returned") | |||
CARBON_KEYWORD_TOKEN(SelfParameter, "self") | |||
CARBON_KEYWORD_TOKEN(SelfType, "Self") | |||
CARBON_KEYWORD_TOKEN(StringTypeLiteral, "String") |
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.
Can you add a TODO or FIXME to document that this is a temporary thing until we have a prelude working?
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.
Can you add a TODO or FIXME to document that this is a temporary thing until we have a prelude working?
I'm adding a vague TODO per your request, but unless I'm misunderstanding the result of #2113 (comment), I don't think this is a "prelude working" problem... This is closer to the decision regarding Type
versus type
on #2360
Reals were mostly handled, but this PR adds storage of them. It also switches a little towards the FloatingPointType semantic from TokenizedBuffer.
While real literals like
1.0
were handled, the type literals were not. This just addsf64
, similar to how I also only supporti32
.The String type literal wasn't used, so I've added support in lexer and parser. Per discussion with @zygoloid String might be renamed based on the newer type literal plan, but it's still String in explorer and the design, so this is just consistent.
The builtin_types.carbon tests the three basic types that are there right now. The test is added to both parser and semantics so that it's clear what the state is in both stages.