Skip to content

Fix #3448 Improve Type inference for IFTs #3449

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

Merged
merged 24 commits into from
Nov 17, 2017
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 9, 2017

We now treat a type as an implicit function type also if it is
a type variable that has an IFT as upper bound in the current constraint.
This means we now create implicit closures if the expected type is
such a type.

Fixes #3448.

Based on #3421.

Don't proceed with implicit search if result type cannot match - the search
will likely by under-constrained, which means that an unbounded number of alternatives
is tried. See strawman-contrib MapDecoratorTest.scala for an example where this happens.
This caused nested ambiguity errors with the next commit.
Propagate implicit ambiguity and divergence failures from implicit arguments
to their callers. In particular, and ambiguity in an implicit argument means
that the whole implicit search is ambiguous. Previously, the local ambiguity
translated to an "implicit not found" on the next outer level, which meant that
another implicit on that level could qualify and the ambiguity would be
forgotten.

The code is still a bit rough and could profit from some refactorings.
This is a temporary fix. iterator-from seems to cause
non-deterministic ambiguity errors. Moving to pending until
we have figured out what goes wrong.
Refactoring of implicits with the aim of clearer code and better error
messages. Here's an example:

    -- Error: implicitSearch.scala:15:12 -------------------------------------------
    15 |    sort(xs)  // error (with a partially constructed implicit argument shown)
       |            ^
       |no implicit argument of type Test.Ord[scala.collection.immutable.List[scala.collection.immutable.List[T]]] was found for parameter o of method sort in object Test.
       |I found:
       |
       |    Test.listOrd[T](Test.listOrd[T](/* missing */implicitly[Test.Ord[T]]))
       |
       |But no implicit values were found that match type Test.Ord[T].
Turns out it's not transitive when combined with the old scheme. So it
should be one or the other. Conservatively, this commit reverts to the old
scheme.
The aim is to have ultimately fewer comparisons. The downside is
that some of the comparisons are now more expensive, because types
have to be compared where they previously weren't.
 - Merge `rankImplicits` and `condense` into one phase
 - Fix handling of ambiguities - the previous unconditional
   abort was wrong, because another alternative might be
   better than both ambiguous choices.
All tests pass, and it is unclear what this is trying to achieve.
We now treat a type as an implicit function type also if it is
a type variable that has an IFT as upper bound in the current constraint.
This means we now create implicit closures if the expected type is
such a type.
@odersky odersky requested a review from smarter November 11, 2017 11:03
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM pending merge of #3421

@odersky odersky merged commit 31a2f52 into scala:master Nov 17, 2017
@allanrenucci allanrenucci deleted the fix-#3448 branch December 14, 2017 19:19
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