Fix is_a? with a union alias resolving to common ancestor#17052
Open
stakach wants to merge 5 commits into
Open
Fix is_a? with a union alias resolving to common ancestor#17052stakach wants to merge 5 commits into
is_a? with a union alias resolving to common ancestor#17052stakach wants to merge 5 commits into
Conversation
…lang#9665) A union used inline inside `is_a?` keeps its members, but the same union behind an alias was resolved to the members' common ancestor, so unrelated sibling types matched. Aliases of unions now keep their members inside `is_a?`, matching inline union behaviour, while still generalizing in value contexts.
A `when Foo | Bar` was compared with `===` against the union value, which generalizes to the members' common ancestor, so unrelated sibling types matched. It now lowers to `is_a?` against the union (like `when Foo, Bar`), while unions of constants (enum members, numeric constants) keep their original value comparison.
e5279a0 to
e54cb92
Compare
Collaborator
|
Did you consider #9665 (comment) |
A `def foo(x : BarBaz)` where `alias BarBaz = Bar | Baz` resolved the alias to the members' common ancestor, so it accepted unrelated sibling types. It now restricts against the union's members, matching the inline `def foo(x : Bar | Baz)`.
Contributor
Author
Yes, however the AST-node-restriction unification would be a much bigger change. |
A `rescue ex : BarBaz` where `alias BarBaz = Bar | Baz` resolved the alias to the members' common ancestor, so it caught unrelated sibling exceptions. It now catches the union's members, matching the inline `rescue ex : Bar | Baz`.
…rystal-lang#9665) The 1.0.0 bootstrap compiler used in CI doesn't narrow a variable assigned inside a compound `&&`/`unless` condition, so the union and rescue handling failed to compile (`undefined method 'first' for Nil`). Lift those assignments out of the conditions; no behaviour change.
Contributor
Author
|
This is ready for review now :) |
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
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.
Resolves #9665.
Issue
A union used inline keeps its members, but the same union behind an alias was resolved to the members' common ancestor, so an unrelated sibling type matched. This affected every place a union flows through type resolution:
Fix
Aliases of unions now preserve their members, matching the inline union form, while still generalizing to the common ancestor in value contexts (
pp BarBaz # => Foois unchanged):is_a?andcase ... when/case ... inagainst a union alias no longer generalize. Awhen Foo | Barnow matches likewhen Foo, Bar; unions of constants (enum members, numeric constants) keep their existing value comparison.def foo(x : BarBaz)) restrict against the union's members.rescue ex : BarBazcatches the union's members rather than their common ancestor.Covers
A | B,Union(A, B), and nested-alias forms.