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

Type inference bug in Scala 3.1.2 #15160

Closed
alexandru opened this issue May 10, 2022 · 11 comments · Fixed by #16001
Closed

Type inference bug in Scala 3.1.2 #15160

alexandru opened this issue May 10, 2022 · 11 comments · Fixed by #16001
Labels
area:implicits related to implicits area:typer itype:enhancement regression This worked in a previous version but doesn't anymore
Milestone

Comments

@alexandru
Copy link

alexandru commented May 10, 2022

I'm unsure if I'm reporting this issue in the right place.

Compiler version

3.1.2 — the issue does not happen in 3.0.x or in 3.1.1.

First bad commit db5956b in #13886

Minimized code

Noticed it while upgrading to 3.1.2 in monix/newtypes:

trait Eq[A] {
  def eqv(a1: A, a2: A): Boolean
}

given stringEq: Eq[String] with {
  def eqv(a1: String, a2: String) = a1 == a2
}

abstract class Newtype[Src] {
  opaque type Type = Src

  protected final def derive[F[_]](using ev: F[Src]): F[Type] = ev
}

object Sample extends Newtype[String] {
  given eq: Eq[Type] = derive
}

Output

[error] -- Error: .../Sample.scala:16:29
[error] 16 |  given eq: Eq[Type] = derive
[error]    |                             ^
[error]    |no implicit argument of type F[String] was found for parameter ev of method derive in class Newtype
[error]    |
[error]    |where:    F is a type variable with constraint <: Eq
[error] one error found

Expectation

The code should compile just fine, as it does with 3.1.1, 3.0.2, or 2.13.x.

@alexandru alexandru added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels May 10, 2022
@bishabosha bishabosha added area:typer regression This worked in a previous version but doesn't anymore and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels May 10, 2022
@smarter smarter added the stat:needs bisection Need to use nightly builds and git bisect to find out the commit where this issue was introduced label May 13, 2022
@smarter
Copy link
Member

smarter commented May 13, 2022

@bishabosha assigned smarter 3 days ago

I won't be able to look at new issues for the next few months, so I recommend not assigning me, but feel free to re-assign me if a bisection points to one of my commits and no one else can figure it out.

@smarter smarter removed their assignment May 13, 2022
@griggt
Copy link
Collaborator

griggt commented May 13, 2022

Bisected to db5956b

@griggt griggt removed the stat:needs bisection Need to use nightly builds and git bisect to find out the commit where this issue was introduced label May 13, 2022
@smarter smarter added the area:implicits related to implicits label May 13, 2022
@odersky
Copy link
Contributor

odersky commented May 13, 2022

This seems to be intentional. In the interest of not allowing compile-times to blow up, we do not perform implicit searches for uninstantiated top-level type variables.

@odersky odersky closed this as completed May 13, 2022
@smarter
Copy link
Member

smarter commented May 13, 2022

While it's uninstantied it can be fully determined by its expected type, so it seems like we could allow it (also this restriction isn't communicated by the error message)

@odersky odersky reopened this May 14, 2022
@odersky
Copy link
Contributor

odersky commented May 18, 2022

I am not sure what the criterion could be to allow it here. We have a type variable that's bounded by Eq, and that's OK. But if it was bounded by a wider type it might well lead to a search explosion. Where do we stop?

Also, note that there is a simple workaround:

given eq: Eq[Type] = derive[Eq]

@odersky odersky removed their assignment May 18, 2022
@odersky
Copy link
Contributor

odersky commented May 18, 2022

I don't think I'll be able to work on this for the time being.

@alexandru
Copy link
Author

👋 from my perspective as a user, if you say there are technical challenges that make this impractical, I believe you. It was surprising to me, as it used to work, and now it doesn't.

If this helps compile times, then that may be a useful trade-off.

But maybe you can improve the error message a little? I'm beginning to be spoiled by Scala 3's cool error message. IDK 🤷‍♂️

@smarter
Copy link
Member

smarter commented May 18, 2022

But if it was bounded by a wider type it might well lead to a search explosion. Where do we stop?

If I understand correctly, the current logic simply does no implicit search if the type passed to it is an uninstantiated type variable, but could we instead force the type variable to be instantiated (recursively, until isUnderspecified returns false) before doing the implicit search?

@odersky
Copy link
Contributor

odersky commented May 18, 2022

If I understand correctly, the current logic simply does no implicit search if the type passed to it is an uninstantiated type variable, but could we instead force the type variable to be instantiated (recursively, until isUnderspecified returns false) before doing the implicit search?

Maybe, but where? You can't very well do it during implicit search, the type variable might be in a nested subgoal and there might be another branch that succeeds. So the only viable scheme is to handle global search failure by looking at the variables and selectively instantiating some of them. Similarly to what we do in typedSelect. That could possibly work. It will be very tricky though to get all the info to the points where it is needed. Once could start improving the error message but even that looks hard.

@smarter
Copy link
Member

smarter commented May 18, 2022

In fact, shouldn't tvarsInParams already take care of selecting F for instantiation in our example?
https://github.com/lampepfl/dotty/blob/72352dc621282a4f9a51eed56e3326d7e80781de/compiler/src/dotty/tools/dotc/typer/Inferencing.scala#L325-L333
It looks like the implementation only looks for type variables in MethodTypes right now, but it seems like it could look in PolyTypes too.

@odersky
Copy link
Contributor

odersky commented May 19, 2022

It looks like the implementation only looks for type variables in MethodTypes right now, but it seems like it could look in PolyTypes too.

Not sure I understand. Type variables bound in polytypes are usually meant to be instantiated by the implicit search, no?

@anatoliykmetyuk anatoliykmetyuk added this to the 3.1.3 milestone May 30, 2022
@odersky odersky removed this from the 3.1.3 milestone May 31, 2022
smarter added a commit that referenced this issue Sep 9, 2022
)

Two improvements for implicit searches involving type variables.

1. We now always add a comment when an implicit search is rejected due
to the "too unspecific" criterion of #13886, commit
[Refine checking for underspecified implicit
queries](db5956b).

There have been quite a few regressions that hit that problem, so it is
good to know immediately what
   the issue is. 

2. There is now a better wildcard approximation of higher-kinded type
applications. This makes several programs (including original #15998)
compile, which were classified as not specific enough before.

Fixes #15998
Fixes #15820
Fixes #15670
Fixes #15160 
Fixes #13986
@Kordyjan Kordyjan added this to the 3.2.2 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:implicits related to implicits area:typer itype:enhancement regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants