Skip to content
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

Represent integer literals as IntLiteral not as i32. #4532

Merged

Conversation

zygoloid
Copy link
Contributor

When an IntLiteral appears as an operand of an if expression, convert it to i32 for now, so that we don't reject things like if cond then 1 else 2 due to having a non-constant value of type IntLiteral.

For tuple indexing expressions such as (a, b).0, convert the index to type IntLiteral, not to type i32. This isn't strictly necessary to do in this PR, but avoids the need to provide an IntLiteral -> i32 implicit conversion for no_prelude tests using this syntax.

sized integer types.

In preparation for changing integer literals to be of `IntLiteral` type.
Conversions from the integer literal type are only permitted when the
value fits within the destination type.

For now, if conversion cannot be checked because the source value is a
symbolic constant, produce a symbolic constant representing the
conversion rather than rejecting it.
Don't hold a Function& after it may have been invalidated by importing.
In `if cond then 1 else 2`, convert to `i32`.
@zygoloid zygoloid added the dependent Depends on another issue/PR label Nov 14, 2024
@zygoloid
Copy link
Contributor Author

Depends on #4526.

Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

Generally LG, pursuant to conversation about file size.

core/prelude/operators/as.carbon Outdated Show resolved Hide resolved
Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

Approving but not merging. The changes here seem fine to me, and I think my one concern will be addressed through #4534.

@zygoloid zygoloid added this pull request to the merge queue Nov 15, 2024
Merged via the queue into carbon-language:trunk with commit bc395eb Nov 15, 2024
8 checks passed
@zygoloid zygoloid deleted the toolchain-intliteral-literals branch November 15, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependent Depends on another issue/PR toolchain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants