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

Fix lowering of a conversion from a type with a pointer value representation to a type with a copy value representation. #4467

Merged

Conversation

zygoloid
Copy link
Contributor

We previously generated a value_bind instruction of the wrong type, resulting in lowering building bad LLVM IR.

Also fix another issue exposed by this change, where we would import constants without marking their types as complete, and then crash in lowering while trying to lower them. This happens in particular for the FunctionTypes of functions in ImplDecls. Address this by skipping lowering for constants with incomplete types.

representation to a type with a copy value representation.

We previously generated a `value_bind` instruction of the wrong type,
resulting in lowering building bad LLVM IR.

Also fix another issue exposed by this change, where we would import
constants without marking their types as complete, and then crash in
lowering while trying to lower them. This happens in particular for the
`FunctionType`s of functions in `ImplDecl`s. Address this by skipping
lowering for constants with incomplete types.
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.

LG

@jonmeow jonmeow added this pull request to the merge queue Nov 1, 2024
Merged via the queue into carbon-language:trunk with commit 32e5212 Nov 1, 2024
10 checks passed
@zygoloid zygoloid deleted the toolchain-convert-class-to-builtin branch November 20, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants