compiler: do not propagate result type to try operand
#22707
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
This commit effectively reverts 9e683f0, and hence un-accepts ziglang#19777. While nice in theory, this proposal turned out to have a few problems. Firstly, supplying a result type implicitly coerces the operand to this type -- that's the main point of result types! But for `try`, this is actually a bad idea; we want a redundant `try` to be a compile error, not to silently coerce the non-error value to an error union. In practice, this didn't always happen, because the implementation was buggy anyway; but when it did, it was really quite silly. For instance, `try try ... try .{ ... }` was an accepted expression, with the inner initializer being initially coerced to `E!E!...E!T`. Secondly, the result type inference here didn't play nicely with `return`. If you write `return try`, the operand would actually receive a result type of `E!E!T`, since the `return` gave a result type of `E!T` and the `try` wrapped it in *another* error union. More generally, the problem here is that `try` doesn't know when it should or shouldn't nest error unions. This occasionally broke code which looked like it should work. So, this commit prevents `try` from propagating result types through to its operand. A key motivation for the original proposal here was decl literals; so, as a special case, `try .foo(...)` is still an allowed syntax form, caught by AstGen and specially lowered. This does open the doors to allowing other special cases for decl literals in future, such as `.foo(...) catch ...`, but those proposals are for another time. Resolves: ziglang#21991 Resolves: ziglang#22633
mlugg
added
the
breaking
|
closes #21319 by just removing the concept :p |
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.
This commit effectively reverts 9e683f0, and hence un-accepts #19777. While nice in theory, this proposal turned out to have a few problems.
Firstly, supplying a result type implicitly coerces the operand to this type -- that's the main point of result types! But for
try, this is actually a bad idea; we want a redundanttryto be a compile error, not to silently coerce the non-error value to an error union. In practice, this didn't always happen, because the implementation was buggy anyway; but when it did, it was really quite silly. For instance,try try ... try .{ ... }was an accepted expression, with the inner initializer being initially coerced toE!E!...E!T.Secondly, the result type inference here didn't play nicely with
return. If you writereturn try, the operand would actually receive a result type ofE!E!T, since thereturngave a result type ofE!Tand thetrywrapped it in another error union. More generally, the problem here is thattrydoesn't know when it should or shouldn't nest error unions. This occasionally broke code which looked like it should work.So, this commit prevents
tryfrom propagating result types through to its operand. A key motivation for the original proposal here was decl literals; so, as a special case,try .foo(...)is still an allowed syntax form, caught by AstGen and specially lowered. This does open the doors to allowing other special cases for decl literals in future, such as.foo(...) catch ..., but those proposals are for another time.Resolves: #21991
Resolves: #22633
This is a breaking change, but it only breaks a feature implemented after 0.13.0, so doesn't need the "release notes" tag.
Also, closes #21319; that enhancement doesn't make sense after this change.