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

Try to instantiate type variables in tryInsertImplicitOnQualifier #13884

Merged
merged 1 commit into from
Nov 15, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 5, 2021

Fixes #13842

What happened in #13842 was that we were not hitting this case in
tryWiden, which is a method in ApproximatingTypeMap that tries
to dealias or widen before propagating a Range outwards:

          case info: SingletonType =>
            // if H#x: y.type, then for any x in L..H, x.type =:= y.type,
            // hence we can replace with y.type under all variances
            reapply(info)

The actual info here was a TypeVar with a singleton type as lower bound. Instantiating
the TypeVar produces a SingletonType and the case applies.

We cannot really instantiate TypeVars here since this is very low-level code. For instance
we might be in a state where the instantiation is provisional and the TyperState is thrown
away. But AsSeenFrom results are cached so we'd have the wrong type in the caches.

What we do instead is add an additional case in tryInsertImplicitOnQualifier which is the
last resort when an application fails. Here, if we can instantiate some type variables in the
qualifier, we try again.

Fixes scala#13842

What happened in scala#13842 was that we were not hitting this case in
`tryWiden`, which is a method in ApproximatingTypeMap that tries
to dealias or widen before propagating a Range outwards:

```scala
          case info: SingletonType =>
            // if H#x: y.type, then for any x in L..H, x.type =:= y.type,
            // hence we can replace with y.type under all variances
            reapply(info)
```
The actual info here was a TypeVar with a singleton type as lower bound. Instantiating
the TypeVar produces a SingletonType and case applies.

We cannot really instantiate TypeVars here since this is very low-level code. For instance
we might be in a state where the instantiation is provisional and the TyperState is thrown
away. But AsSeenFrom results are cached so we'd have the wrong type in the caches.

What we do instead is add an additional case in `tryInsertImplicitOnQualifier` which is the
last resort when an application fails. Here, if we can instantiate some type variables in the
qualifier, we try again.
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.

But AsSeenFrom results are cached so we'd have the wrong type in the caches.

But we make sure to not cache asSeenFrom when the prefix isProvisional: https://github.com/lampepfl/dotty/blob/4ad416fc3153c953835067e25c6a12309f714256/compiler/src/dotty/tools/dotc/core/Denotations.scala#L137 and an instantiation of a type variable won't set isProvisional to false unless the instantiation is permanent, so I think this would be fine?

@odersky
Copy link
Contributor Author

odersky commented Nov 10, 2021

I still think TypeMap is too low-level to be in the business of instantiating type variables. TypeMaps should not have observable side effects.

@smarter smarter merged commit 6e3e34d into scala:master Nov 15, 2021
@smarter smarter deleted the fix-13842 branch November 15, 2021 09:55
@Kordyjan Kordyjan added this to the 3.1.2 milestone Aug 2, 2023
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.

Strange type inference failure for module types
3 participants