Skip to content

GADT type unification chain length limitation #11682

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

Closed
Linyxus opened this issue Mar 10, 2021 · 6 comments · Fixed by #11785
Closed

GADT type unification chain length limitation #11682

Linyxus opened this issue Mar 10, 2021 · 6 comments · Fixed by #11785

Comments

@Linyxus
Copy link
Contributor

Linyxus commented Mar 10, 2021

Compiler version

3.0.0-M4-bin-SNAPSHOT, 3.0.0-RC1

Minimized code

object Test {
  trait Tag[T]

  // works: Int = Z = Y = T
  def func1[Z >: Int <: Int, Y >: Z <: Z, T >: Y <: Y]: T = {
    0
  }

  // works: Int = Z = Y = T
  def func2[Z >: Int <: Int, Y >: Z <: Z, T]: Tag[T] => T = {
    case _ : Tag[Y] => 0
  }

  // works: Int = Z = Y = X = T
  def func3[Z >: Int <: Int, Y >: Z <: Z, X >: Y <: Y, T >: X <: X]: T = {
    0
  }

  // expect: Int = Z = Y = X = T
  // actual: can not derive T = Int
  def func4[Z >: Int <: Int, Y >: Z <: Z, X >: Y <: Y, T]: Tag[T] => T = {
    case _ : Tag[X] => 0  // error
  }
}

Output

-- [E007] Type Mismatch Error: foobar/gadt-poly.scala:24:23 --------------------
24 |    case _ : Tag[X] => 0
   |                       ^
   |                       Found:    (0 : Int)
   |                       Required: T
1 error found

Expectation

It should infer the constraint T = X = Y = Z = Int.

@abgruszecki

@Kordyjan Kordyjan assigned abgruszecki and unassigned abgruszecki Mar 10, 2021
@Kordyjan Kordyjan changed the title Can not correctly infer GADT constraints in some cases Type unification chain length limitation Mar 10, 2021
@Kordyjan
Copy link
Contributor

Kordyjan commented Mar 10, 2021

@Linyxus Your example shows that the problem has a scope broader than just GADTs and is a result of the limitation of the current typechecker.

@smarter
Copy link
Member

smarter commented Mar 10, 2021

@Kordyjan are you sure? The only example with // error involves GADT constraints.

@Kordyjan
Copy link
Contributor

@smarter: Now I see I misunderstood the sample. It indeed seems to be a problem with GADTs constraints.

@smarter smarter changed the title Type unification chain length limitation GADT type unification chain length limitation Mar 10, 2021
@abgruszecki abgruszecki self-assigned this Mar 10, 2021
@abgruszecki
Copy link
Contributor

abgruszecki commented Mar 10, 2021

@smarter I guess this issue doesn't remind you of anything similar in type inference? I'd be somewhat surprised if there was anything specific to the GADT logic that would make us handle correctly instantiation chains of length 3, but not of length 4, so my intuition is that there's something in constraint handling to blame. Ofc, it's entirely possible that I'm (still) misusing constraint handling.

@smarter
Copy link
Member

smarter commented Mar 10, 2021

No, this doesn't ring a bell, I thought it might be a deep subtype issue but running with -Yno-deep-subtypes does not make a difference.

@abgruszecki
Copy link
Contributor

Ok, I took a look and it indeed seems like the problem is in ConstraintHandling. The traces are here: https://gist.github.com/abgruszecki/b6cc78b3f38aeec04f9d8d7160a6ee3b

It seems that for the case that works, when we initially add the constraints, we have:

added to constraint: [[Z(param)1 >: Int <: Int, Y(param)1 >: Z(param)1 <: Z(param)1, T(param)2] => Any] type Z, type Y, type T
Constraint(
 uninstantiated variables: Z(param)1, T(param)2
 constrained types: [Z(param)1 >: Int <: Int, Y(param)1 >: Z(param)1 <: Z(param)1, T(param)2] => Any
 bounds: 
     Z(param)1 >: Int <: Int
     Y(param)1 := Int
     T(param)2
 ordering: 
     Y(param)1 <: Z(param)1
)
type Z:  >: Int <: Int
type Y:  >: Int <: Z
type T: 

And for the case that doesn't work, we have:

added to constraint: [[Z(param)1 >: Int <: Int, Y(param)1 >: Z(param)1 <: Z(param)1, X(param)1 >: Y(param)1 <: Y(param)1, 
  T(param)2
] => Any] type Z, type Y, type X, type T
Constraint(
 uninstantiated variables: Z(param)1, X(param)1, T(param)2
 constrained types: 
  [Z(param)1 >: Int <: Int, Y(param)1 >: Z(param)1 <: Z(param)1, X(param)1 >: Y(param)1 <: Y(param)1, 
    T(param)2
  ] => Any
 bounds: 
     Z(param)1 >: Int <: Int
     Y(param)1 := Int
     X(param)1 <: AnyKind
     T(param)2
 ordering: 
     Y(param)1 <: X(param)1, Z(param)1
)
type Z:  >: Int <: Int
type Y:  >: Int <: Z
type X:  <: AnyKind
type T: 

Note that X is only <: AnyKind, where it probably should be X := Int. That seems to be the source of the problem.

Linyxus added a commit to Linyxus/dotty that referenced this issue Mar 17, 2021
Linyxus added a commit to Linyxus/dotty that referenced this issue Mar 17, 2021
michelou pushed a commit to michelou/scala3 that referenced this issue Mar 22, 2021
smarter added a commit to dotty-staging/dotty that referenced this issue Apr 29, 2022
…arams

`order` takes `current` as input and returns a constraint set that subsumes both
`current` and `param1 <: param2`, but it's an instance method because it relies
on `this` to determine if `current` is used linearly such that we can reuse its
backing arrays instead of copying them. However, the implementation of `order`
mistakenly returned `this` and called methods on `this` instead of `current`.
This lead to issues like scala#11682 but that was compensated by logic inserted
in ConstraintHandling#addToConstraint which we can now remove.

Fixing this also required fixing an unrelated issue in avoidLambdaParams to
prevent a regression in tests/pos/i9676.scala: we shouldn't avoid a lambda param
under its own binder even if it is in `comparedTypeLambdas`, the sequence of
operations where this happens is:

[A] =>> List[A] <:< [A] =>> G[A]
  // comparedTypeLambdas ++= ([A] =>> List[A], [A] =>> G[A])
  List[A] <:< G[A]
    [A] =>> List[A] <:< G
      // previously, avoidLambdaParams([A] =>> List[A]) = [A] =>> List[Any],
      // now it leaves the type lambda alone.

We end up checking `[A] =>> List[A] <:< G` instead of just `List <:< G` because
of `ensureLambdaSub` in `compareAppliedTypeParamRef`. I'm not sure if this is
actually needed, but I decided to not disturb that code too much for now.
bishabosha pushed a commit to dotty-staging/dotty that referenced this issue Oct 18, 2022
…arams

`order` takes `current` as input and returns a constraint set that subsumes both
`current` and `param1 <: param2`, but it's an instance method because it relies
on `this` to determine if `current` is used linearly such that we can reuse its
backing arrays instead of copying them. However, the implementation of `order`
mistakenly returned `this` and called methods on `this` instead of `current`.
This lead to issues like scala#11682 but that was compensated by logic inserted
in ConstraintHandling#addToConstraint which we can now remove.

Fixing this also required fixing an unrelated issue in avoidLambdaParams to
prevent a regression in tests/pos/i9676.scala: we shouldn't avoid a lambda param
under its own binder even if it is in `comparedTypeLambdas`, the sequence of
operations where this happens is:

[A] =>> List[A] <:< [A] =>> G[A]
  // comparedTypeLambdas ++= ([A] =>> List[A], [A] =>> G[A])
  List[A] <:< G[A]
    [A] =>> List[A] <:< G
      // previously, avoidLambdaParams([A] =>> List[A]) = [A] =>> List[Any],
      // now it leaves the type lambda alone.

We end up checking `[A] =>> List[A] <:< G` instead of just `List <:< G` because
of `ensureLambdaSub` in `compareAppliedTypeParamRef`. I'm not sure if this is
actually needed, but I decided to not disturb that code too much for now.
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants