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

Compiler: implement autocasting in a better way #9501

Merged

Conversation

asterite
Copy link
Member

I thought of a better, more resilient way to implement autocasting. This removes the existing hacks around it.

I added a big chunk of comment explaining how this is currently done, and what with_literals and analyze_all means, with a bunch of examples.

Makes #9492 obsolete.

Copy link
Member

@jhass jhass left a comment

Choose a reason for hiding this comment

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

🎉

src/compiler/crystal/semantic/method_lookup.cr Outdated Show resolved Hide resolved
@jwoertink
Copy link
Contributor

Nice! Does this also fix #8973 ?

@asterite
Copy link
Member Author

asterite commented Jun 18, 2020

@jwoertink No, that has nothing to do with autocasting. #8973 can't be fixed without a major overhaul to how methods are looked up. And even then, I'm not sure how it should work. So it needs a bit more "thinking" time before it can be fixed. Not from the implementation side, but the actual semantics.

@asterite
Copy link
Member Author

CI is happy 🎉

@asterite
Copy link
Member Author

asterite commented Jul 3, 2020

@waj @bcardiff ping! It would be nice to have this in 1.0

@waj waj added this to the 1.0.0 milestone Jul 3, 2020
@asterite asterite force-pushed the feature/automatic-casting-for-real branch from 9e9e9d2 to e423bcc Compare July 3, 2020 20:57
@asterite
Copy link
Member Author

asterite commented Jul 3, 2020

@waj Thanks! I rebased this branch against master just to make sure CI passes... CI also seemed stuck.

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.

5 participants