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

Eta expand functions with optional parameters when resolving implicits #493

Closed
wants to merge 1 commit into from

Conversation

TimWhiting
Copy link
Collaborator

@TimWhiting TimWhiting commented Apr 11, 2024

When using a function with an optional parameter as implicit, the types do not unify. Eta-expanding the function fixes the issue. This is particularly important due to double/show having the default parameter for precision.

@TimWhiting TimWhiting marked this pull request as ready for review April 11, 2024 02:05
daanx added a commit that referenced this pull request Sep 13, 2024
…match with a function type without optional parameters. This also addresses PR #493 (I think)
@daanx
Copy link
Member

daanx commented Sep 13, 2024

Hi Tim -- thanks for the PR, I think it looks good. However, I have generalized this idea a bit and added code to automatically eta-expand any variable argument that has optional parameters but the expected type does not. For example, in:

fun plus(x:int,y:int,z:int=1)
  x+y+z

pub fun main()
  [1,2,3].foldl(0,plus)

I think the commit 123d3e7 also handles what this PR is doing -- but I'm not 100% sure; maybe you can see if it covers your use cases?

@TimWhiting
Copy link
Collaborator Author

@daanx
This still fails: stack run koka -- -e test/syntax/run/tail. Also apparently the additional config.json options in the run directory are not replacing or being added the options in the parent directory, so the run tests aren't actually being run.

@daanx
Copy link
Member

daanx commented Sep 18, 2024

Ah, I think this is a different problem -- namely, the main that is generated does a println on a float64 but it doesn't import std/num/float64. This is something we should fix as well.

@daanx
Copy link
Member

daanx commented Sep 19, 2024

I think the import problem is fixed now in the latest dev (commit f05b93c) . (the tail tests work)

@TimWhiting TimWhiting closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants