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

[BUG] Automatic casting of symbols with overloads and named arguments broken #7578

Closed
straight-shoota opened this issue Mar 23, 2019 · 4 comments · Fixed by #7747
Closed
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic

Comments

@straight-shoota
Copy link
Member

There is a bug with automatic casting of symbols to enum values when using named arguments defined in multiple overloads of a method.
Reproducible example:

enum Foo
  FOO
end

def foo(unused, foo : Foo)
end

def foo(foo : Foo)
end

foo(foo: :FOO) # ambiguous call, implicit cast of :FOO matches all of Foo

https://carc.in/#/r/6k06

Citing @asterite from #6314 (comment) :

If I remember correctly, the problem is in these lines:

defs_matching_args_size = defs.select do |a_def|
min_size, max_size = a_def.min_max_args_sizes
min_size <= real_args_size <= max_size
end

The logic is incorrect, it only works if there are no named arguments and all the defs that match the signature, ignoring type restrictions, is not well computed. Someone should fix that logic and it will start working well... I think. But I don't have time to tackle this.

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Mar 23, 2019
@Maroo-b
Copy link
Contributor

Maroo-b commented Apr 1, 2019

@straight-shoota I can work on this, the output from the shared example should be the execution of method def foo(foo : Foo) without raising any error?

@Maroo-b
Copy link
Contributor

Maroo-b commented Apr 7, 2019

I spent a lot of time on this but I didn't manage to find where this method is called twice: https://github.com/crystal-lang/crystal/blob/master/src/compiler/crystal/types.cr#L1358 I assume that what was causing the issue for autocasting Symbols.
I may be wrong but I was following the logic so far and call_error is responsible for triggering the lookup with literals (for autocasting) by raising this exception: https://github.com/crystal-lang/crystal/blob/master/src/compiler/crystal/semantic/call_error.cr#L249 (so far no issue found in that file)
For reference those PRs where useful:

@asterite
Copy link
Member

asterite commented Apr 7, 2019

Hey, I think I know why it happens but I don't have time to fix it nor explain it. Sorry.

@Maroo-b
Copy link
Contributor

Maroo-b commented Apr 8, 2019

I'll try to investigate more anyway it may help me to understand the code base.

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:semantic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants