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

Infer function parameter types from partially eta-expanded methods (backport #7316 and #7333) #7340

Merged
merged 5 commits into from Nov 2, 2018

Conversation

Projects
None yet
5 participants
@adriaanm
Copy link
Member

adriaanm commented Oct 15, 2018

Backport a fix for scala/bug#9745 (#7316) and improve type inference for partial eta-expansion (#7333)

This improves inference of function parameter types for partially eta-expanded methods (i.e., when only some arguments are omitted using _)

Example from the test, given def repeat(x: Int, y: String) = y * x, partial eta-expansion recovers fun param types from the definition of the selected method:

val sayAaah = repeat(_, "a") // place holder syntax
val thrice = x => repeat(3, x) // explicit version
val repeatFlip = (x, y) => repeat(y, x) // explicit version, two params

@scala-jenkins scala-jenkins added this to the 2.12.8 milestone Oct 15, 2018

adriaanm added some commits Oct 5, 2018

[backport] Factor typedFunction.
Let's create a fast path for after typer, when
we already know which SAM to target, and all
parameter types are known

Backported from b2edce8
[backport] Do less in typedFunction after typer
Do more for typedFunction of eta-expanded method:
keep typed method selection (fix scala/bug#9745, follow up for #6007)

Test case taken from original PR.

Backported from 64d4c24
[backport] Pull out `argsResProtosFromFun`,
Drop impossible error check.

We always produced `numVparams` argument prototypes, so there
was no way we would ever call WrongNumberOfParametersError.

Backported from a9f8c14
[backport] Recover fun param types from (partial) eta-expansion
Generalize function parameter type inference to allow
any subset (including permutation) of parameters with
unknown types to be inferred from the method they are
applied to.

Before, we required all method arguments to come from
the function's vparams, in order.

Example:

```
scala> def repeat(x: Int, y: String) = y * x
repeat: (x: Int, y: String)String

scala> val sayAaah = repeat(_, "a")
sayAaah: Int => String

scala> val thrice = x => repeat(3, x)
thrice: String => String

scala> val repeatFlip = (x, y) => repeat(y, x)
repeatFlip: (String, Int) => String
```

Backported from 967ab56

@adriaanm adriaanm force-pushed the adriaanm:eta-partial-212 branch from ac0dbb7 to 93cd804 Oct 15, 2018

@lrytz

This comment has been minimized.

Copy link
Member

lrytz commented Oct 19, 2018

It's not a small diff, and touches a quite heavily used and non-trivial part of the type checker. On the other hand, the fact that it's heavily used (in 2.13, exercised by collections) gives it a good coverage. So I'm +1.

[backport] Review feedback
(cherry picked from commit a38f306)
@adriaanm

This comment has been minimized.

Copy link
Member Author

adriaanm commented Oct 19, 2018

Note that I dropped one behavior change from the 2.13 version: we still report errors in bodies of functions that have params with missing types.

@retronym
Copy link
Member

retronym left a comment

I'm happy to align 2.12 with 2.13 here.

It would be great to write a short doc describing the various changes and fixes to type inference of lambdas so that @niktrop et al can bring them into IntelliJ.

@lrytz lrytz merged commit 5fca7b2 into scala:2.12.x Nov 2, 2018

4 checks passed

cla @adriaanm signed the Scala CLA. Thanks!
Details
combined All previous commits successful.
continuous-integration/travis-ci/pr The Travis CI build passed
Details
validate-main [1219] SUCCESS. Took 53 min.
Details
@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Nov 2, 2018

I think this is release-note worthy for 2.12.8, so that's another reason for the PR description to be updated to describe the user-facing parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.