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

Adding support of path-dependent GADT reasoning #14754

Closed
wants to merge 56 commits into from

Conversation

Linyxus
Copy link
Contributor

@Linyxus Linyxus commented Mar 23, 2022

Previously, GADT reasoning only works on type parameters. This PR supports GADT reasoning for path-dependent types (that is, type members addressed from stable paths).

It mainly makes the following two things possible:

  1. It allows us to utilize the GADT constraints of type members implied by the pattern matching. Example:
type typed[E <: Expr, V] = E & { type T = V }

trait Expr { type T }
case class LitInt(x: Int) extends Expr { type T = Int }
case class Add(e1: Expr typed Int, e2: Expr typed Int) extends Expr { type T = Int }
case class LitBool(x: Boolean) extends Expr { type T = Boolean }
case class Or(e1: Expr typed Boolean, e2: Expr typed Boolean) extends Expr { type T = Boolean }

def eval(e: Expr): e.T = e match
  case LitInt(x) => x  // for example: here we can derive the bound e.T = Int
  case Add(e1, e2) => eval(e1) + eval(e2)
  case LitBool(b) => b
  case Or(e1, e2) => eval(e1) || eval(e2)
  1. It enables the compiler to record GADT bounds for constrainable path-dependent types even if their path is not pattern-matched against. Example:
trait Expr[+X]
case class LitInt(x: Int) extends Expr[Int]

trait TypeTag { type A }

def foo(p: TypeTag, e: Expr[p.A]): p.A = e match
  case LitInt(x) => x  // here we derive the bounds p.A >: Int, just like what we do for type parameters

Fixes #15868.

@s5bug
Copy link

s5bug commented Jun 16, 2022

What's required to push this PR to the finish line? I have a system similar to

trait T[X]
case object Foo extends T[Unit]

trait AsParam[L] {
  val tl: T[L]
}

def testParam[A](ap: AsParam[A]): Unit =
  ap.tl match {
    case Foo => println(summon[A =:= Unit])
    case _ => ()
  }

that I'd like to refactor into the equivalent of (the currently non-compiling on mainline Dotty)

trait AsMember {
  type L
  val tl: T[L]
}

def testMember(am: AsMember): Unit =
  am.tl match {
    case Foo => println(summon[am.L =:= Unit])
    case _ => ()
  }

so that methods can return AsMember instead of AsParam[?], which requires such like

val asParamQ: AsParam[?] = someFunction
asParamQ match {
  case apt: AsParam[t] => apt.tl match {
    case Foo => println(summon[t =:= Unit])
    case _ => ()
  }
}

to consume (which does happen to compile on mainline).

@Linyxus
Copy link
Contributor Author

Linyxus commented Jun 16, 2022

Hi! Glad to see this PR can benefit real-world development. It is currently blocked by #14776. This is a bug discovered when developing this PR and the tests currently broken would work after fixing this issue.

@Linyxus
Copy link
Contributor Author

Linyxus commented Jun 16, 2022

Just verified that the following snippet compiles on this branch. :)

trait T[X]
case object Foo extends T[Unit]

trait AsMember {
  type L
  val tl: T[L]
}

def testMember(am: AsMember): Unit =
  am.tl match {
    case Foo => println(summon[am.L =:= Unit])
    case _ => ()
  }

Linyxus added a commit to Linyxus/dotty that referenced this pull request Jul 15, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
Linyxus added a commit to Linyxus/dotty that referenced this pull request Jul 22, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
Linyxus added a commit to Linyxus/dotty that referenced this pull request Jul 22, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
Linyxus added a commit to Linyxus/dotty that referenced this pull request Jul 22, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
Linyxus added a commit to Linyxus/dotty that referenced this pull request Jul 22, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
Linyxus added a commit to Linyxus/dotty that referenced this pull request Jul 22, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
@Linyxus
Copy link
Contributor Author

Linyxus commented Jul 22, 2022

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/14754/ to see the changes.

Benchmarks is based on merging with main (634c580)

Linyxus added a commit to Linyxus/dotty that referenced this pull request Aug 9, 2022
Thanks to the path-dependent GADT reasoning logic, these limitation errors can
be dropped from the testcases. See scala#14754.
@Linyxus
Copy link
Contributor Author

Linyxus commented Aug 10, 2022

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/14754/ to see the changes.

Benchmarks is based on merging with main (e560c2d)

@Linyxus
Copy link
Contributor Author

Linyxus commented Sep 21, 2022

Hey Alex! @abgruszecki I have finished one round of PR polishing. I have done the following things:

  • I have refactored the interface of path aliasing constraints. It is now more modular.
  • I fixed the typeMemberTouched bug we discussed before, companied with a test. I also refactored the recursive calls to constrainPatternType by moving the logic into a recur function. So now we do not have typeMemberTouched argument at toplevel.
  • Fixed one bug related to pattern path.
  • Added tons of documentation.

There are one thing we discussed before but not in the review comments yet, so I am not sure whether I should do it: refactoring the states related to pattern and scrutinee paths into another class in Context, instead of keeping them in GadtConstraint. I am not sure what we should do here because the states for recording the scrutinee and the pattern paths are closely related to GADT reasoning and the way we implement path-dependent GADT. But maybe refactor them into another class will make things more clear and modular?

@ckipp01
Copy link
Member

ckipp01 commented May 10, 2023

Hey @Linyxus are you willing to give this a rebase? If so, I think we can try and get this reviewed.

@Linyxus
Copy link
Contributor Author

Linyxus commented May 11, 2023

Similarly, we will wait for a refactorization of GADT (#15928) before proceeding this feature. I will close this for now and go back to it after the refactorization is done.

@Linyxus Linyxus closed this May 11, 2023
@LPTK
Copy link
Contributor

LPTK commented Jan 15, 2024

#15928 was closed. Is there any future for the changes in this PR?

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.

GADT parameter ordering is not recorded during registration
6 participants