Propagate errors in import #5728
Merged
Conversation
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
jonmeow
approved these changes
Jun 25, 2025
Comment on lines
146
to
148
| fn F() { | ||
| //@dump-sem-ir-begin | ||
| WithError(()); |
There was a problem hiding this comment.
Noting that the generated IR doesn't include any errors (but I do note the absence of a call), I'd wonder if something like a let might show the transmission:
Suggested change
| fn F() { | |
| //@dump-sem-ir-begin | |
| WithError(()); | |
| fn F() { | |
| //@dump-sem-ir-begin | |
| let x: () = WithError(()); |
I think that'll yield:
+// CHECK:STDOUT: %x: %empty_tuple.type = bind_name x, <error> [concrete = <error>]
There was a problem hiding this comment.
Thanks, yep it does, exactly.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 2, 2025
…5729) Currently if the first operand contains an error, we will return error, even though the second operands contains a runtime, and it has a stronger priority (the phase always goes up if possible). Import is only allowed on instructions with compile-time values, so we crash if we ever try to import a runtime value. Importable instructions must diagnose unexpected runtime values and produce errors in the semir from which they would be imported so that runtime values are never imported by another semir. If we had an instruction where you had an error value from the first operand, and runtime from the second, and we imported it: - Before #5728 we would crash in import, but only because we treated errors as runtime - After #5728 we would import ErrorInst because we propagate errors. This is desirable for cases with compile-time values and errors present only. - After this PR, we would crash again, cuz you're importing a runtime thing. This change means that instructions containing an `InstConstantKind::Never` instruction like`ValueParam` will consistently evaluate to a runtime value, even if there are errors present. This is visible in the `BindName` instructions changing in the semir, where they became constant `ErrorInst` values previously but no longer do.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
AddImportedInstructionwas turning errors in an instruction into a Runtime constant value instead of an Error, which led to crashes when importing an instruction that had an error inside it somewhere.Fixes #5726