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 several compile-time operations on generic instance metaclasses #11101

Merged

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Aug 17, 2021

An assortment of fixes that improve the correctness of generic instance metaclass types (e.g. Array(Int32).class, Enumerable(String).class).

  • Disallows T.class.class as a distinct type when T is a generic instance. The same type expression will produce Class just like other kinds of metaclasses.
  • Gives proper superclasses to the metaclasses of generic class instances. This is reflected by TypeNode#superclass, where the same code would currently produce nil twice: (#ancestors already worked correctly prior to this PR)
    class Foo(T); end
    class Bar(T) < Foo(T); end
    
    {% Foo(Int32).class.superclass %} # => Reference.class
    {% Bar(Int32).class.superclass %} # => Foo(Int32).class
  • Makes it so that T.class <= U.class if and only if T <= U. Previously this was not computed properly when either type is a generic instance; T.class was regarded as a normal type in Crystal::Type#implements?, so it completely ignored subtyping due to generics, causing the following lines to produce false:
    {% Bar(Int32).class < Bar.class %} # => true
    {% Bar(Int32).class < Foo.class %} # => true
    {% Bar.class < Foo.class %}        # => true
    
    # If `Foo(T)` were a module and included by `Bar(T)`, even the following
    # would currently produce `false`
    {% Bar(Int32).class < Foo(Int32).class %} # => true
  • Makes it so that T.class < U.class if T inherits from U (module inclusion does not count), or if T is a generic instance of U. Previously this was not computed properly when either type is a generic instance; T.class was regarded as a normal type in Crystal::Type#implements?, so it completely ignored subtyping due to generics, causing the following lines to produce false:
    {% Bar(Int32).class < Bar.class %} # => true
    {% Bar(Int32).class < Foo.class %} # => true
    {% Bar.class < Foo.class %}        # => true

Does not affect #11068. Also does not affect Class#<= (subtyping comparisons between runtime metaclasses).

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:generics labels Aug 17, 2021
@HertzDevil
Copy link
Contributor Author

For #11110 this will fix the subtyping check, but the ancestors of generic module instance metaclasses will still include Object.class:

module Foo(T); end

{% Foo(Int32).class <= Object.class %} # => false
{% Foo(Int32).class.ancestors %}       # => [Object.class, Class, Value, Object]

@HertzDevil HertzDevil marked this pull request as draft August 23, 2021 11:05
@HertzDevil HertzDevil marked this pull request as ready for review August 23, 2021 11:08
@HertzDevil
Copy link
Contributor Author

HertzDevil commented Aug 23, 2021

It seems we copied Ruby for not introducing a subtyping relation between T.class and U.class when T includes the module U (see #11119), so 12cdcc6 is replaced by 0b5ac18.

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.

Looks great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:generics topic:lang:type-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants