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

Never resolve free variables as types during overload ordering #11973

Merged

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Apr 5, 2022

In the following snippet, the Bar in x : Bar is resolved as a type even though it is a free variable, and since Bar < Foo, the Bar overload is ordered first:

class Foo; end
class Bar < Foo; end

def foo(x : Foo); 1; end
def foo(x : Bar) forall Bar; end

foo(Foo.new) # => nil

Bar forall Bar accepts types that are not subtypes of Foo, so the overload order should be untouched here.

Similarly, in the following snippet, the two defs are redefinitions because the restrictions have the same name, even though only one of them is a free variable and the two are definitely distinguishable:

class Foo; end

def foo(x : Foo) forall Foo; end
def foo(x : Foo); end

foo(3) # Error: no overload matches 'foo' with type Int32

This PR makes it so that those free variables are never resolved as types; for the purposes of overload ordering, they are considered undefined types, and the non-free Paths come first. Most free variables have only one letter and most type names don't, so the impact should be very small in practice.

The first commit here is also necessary when we eventually deal with bounded free variables.

Nothing changes if both Paths are free variables (they are already somewhat broken before and nobody really knows how to fix them yet, for example #10042).

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Apr 5, 2022
@HertzDevil HertzDevil added this to the 1.6.0 milestone Sep 2, 2022
@straight-shoota straight-shoota merged commit 6ce7f3e into crystal-lang:master Sep 5, 2022
@HertzDevil HertzDevil deleted the bug/free-variable-type-lookup branch September 6, 2022 01:46
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. kind:refactor topic:compiler:semantic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants