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

Define type filtering through an intersection operation #10781

Merged

Conversation

HertzDevil
Copy link
Contributor

Refactors type filtering operations (used for is_a? and as) into a new internal method Crystal::Type.common_descendent, a dual to common_ancestor, in a similar vein as #10527. This is a prerequisite for eventually implementing #2404.

The first commit mostly copies the type-versus-type portion of Type#restrict to common_descendent, subsequent commits are further refactors. There is a slight performance boost here because the new intersection no longer needs a MatchContext object (the context is never needed unless AST node restrictions are involved, e.g. to access self or bind free variables). Since type restrictions must behave like AST node restrictions for the time being, the old logic in restrictions.cr remains intact.

The new method uses #implements? where the restriction code would use #restriction_of?, and once again the context object is unneeded here. There is exactly one spec that fails due to this change, and I have confirmed that it would be fixed by #10779.

This PR is purely a refactor; it does not make type filtering commutative yet, and it is relatively apparent where commutativity fails. It is up to a second PR to turn this method into a commutative greatest common descendent operation.

Copy link
Member

@asterite asterite left a comment

Choose a reason for hiding this comment

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

I don't know if we are in a PR freeze already, but otherwise feel free to merge this.

@HertzDevil
Copy link
Contributor Author

5c4b946 contains #10883 and the corresponding post-refactor quick fix.

@straight-shoota straight-shoota merged commit f1cce3a into crystal-lang:master Jul 25, 2021
@HertzDevil HertzDevil deleted the refactor/restrict-type3 branch July 25, 2021 16:29
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.

None yet

3 participants