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 check_type_allowed_as_proc_argument to show the type name #10688

Conversation

straight-shoota
Copy link
Member

All other call sites of check_type_can_be_stored mention the respective type in the message, only for proc arguments this was missing.

Specs fail because Class type restriction sometimes reports as Object. Not sure about the reason or if that's intended, but it looks unexpected to me.

->(x : Class) { } # Can't use Object as a Proc argument type

def foo(&block : Class ->)
end

foo {} # Can't use Class as a Proc argument type

def foo(x)
end

->foo(Class) # Can't use Object as a Proc argument type

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels May 7, 2021
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 good! Though CI is failing

@straight-shoota
Copy link
Member Author

Yes, I mentioned that in the OP. Any idea why Class reports as Object?

@asterite
Copy link
Member

asterite commented Jun 2, 2021

It's a bug, but I can't remember why it happens.

Also, the original intention of the code was to show the invalid type inside a union type, but I guess that was unused.

@HertzDevil
Copy link
Contributor

Class's instance type is Object, and somewhere Class is treated like a metaclass. #10590 also observed this.

@straight-shoota
Copy link
Member Author

Ah thanks, I had a vague idea that something about that had come up somewhere lately. I'll adapt the specs for now.

@straight-shoota straight-shoota added this to the 1.1.0 milestone Jun 15, 2021
@asterite asterite merged commit 39e5efc into crystal-lang:master Jun 18, 2021
@straight-shoota straight-shoota deleted the fix/compiler-proc-argument-storable-error branch June 18, 2021 12:54
@straight-shoota straight-shoota changed the title Fix check_type_allowed_as_proc_argument to show show type name Fix check_type_allowed_as_proc_argument to show the type name Jun 29, 2021
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 this pull request may close these issues.

3 participants