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

Avoid bidirectional GADT typebounds from fullBounds #15683

Merged
merged 5 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions compiler/src/dotty/tools/dotc/core/GadtConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dotc
package core

import Contexts.*, Decorators.*, Symbols.*, Types.*
import NameKinds.UniqueName
import config.Printers.{gadts, gadtsConstr}
import util.{SimpleIdentitySet, SimpleIdentityMap}
import printing._
Expand Down Expand Up @@ -71,16 +72,31 @@ class GadtConstraint private (
externalize(constraint.nonParamBounds(param)).bounds

def fullLowerBound(param: TypeParamRef)(using Context): Type =
constraint.minLower(param).foldLeft(nonParamBounds(param).lo) {
(t, u) => t | externalize(u)
val self = externalize(param)
constraint.minLower(param).foldLeft(nonParamBounds(param).lo) { (acc, p) =>
externalize(p) match
case tp: TypeRef
// drop any lower param that is a GADT symbol
// and is upper-bounded by a non-Any super-type of the original parameter
// e.g. in pos/i14287.min
// B$1 had info <: X and fullBounds >: B$2 <: X, and
// B$2 had info <: B$1 and fullBounds <: B$1
// We can use the info of B$2 to drop the lower-bound of B$1
// and return non-bidirectional bounds B$1 <: X and B$2 <: B$1.
if tp.name.is(UniqueName) && !tp.info.hiBound.isExactlyAny && self <:< tp.info.hiBound => acc
Copy link
Member

Choose a reason for hiding this comment

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

Can we use tp.symbol.isPatternBound instead of relying on the name kind?

Copy link
Member

Choose a reason for hiding this comment

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

Also, would checking self =:= tp.info.hiBound be enough for all our test cases? That would remove the need for isExactlyAny which I'm a bit wary of (what if the symbols involved have another common upper-bound like AnyRef?)

case tp => acc | tp
}

def fullUpperBound(param: TypeParamRef)(using Context): Type =
constraint.minUpper(param).foldLeft(nonParamBounds(param).hi) { (t, u) =>
val eu = externalize(u)
// Any as the upper bound means "no bound", but if F is higher-kinded,
// Any & F = F[_]; this is wrong for us so we need to short-circuit
if t.isAny then eu else t & eu
val self = externalize(param)
constraint.minUpper(param).foldLeft(nonParamBounds(param).hi) { (acc, u) =>
externalize(u) match
case tp: TypeRef // same as fullLowerBounds
if tp.name.is(UniqueName) && !tp.info.loBound.isExactlyNothing && tp.info.loBound <:< self => acc
case tp =>
// Any as the upper bound means "no bound", but if F is higher-kinded,
// Any & F = F[_]; this is wrong for us so we need to short-circuit
if acc.isAny then tp else acc & tp
}

def externalize(tp: Type, theMap: TypeMap | Null = null)(using Context): Type = tp match
Expand Down
7 changes: 7 additions & 0 deletions tests/pos/i14287.min.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
enum Foo[+F[_]]:
case Bar[B[_]](value: Foo[B]) extends Foo[B]

class Test:
def test[X[_]](foo: Foo[X]): Foo[X] = foo match
case Foo.Bar(Foo.Bar(x)) => Foo.Bar(x)
case _ => foo
16 changes: 16 additions & 0 deletions tests/pos/i14287.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// scalac: -Yno-deep-subtypes
enum Free[+F[_], A]:
case Return(a: A)
case Suspend(s: F[A])
case FlatMap[F[_], A, B](
s: Free[F, A],
f: A => Free[F, B]) extends Free[F, B]

def flatMap[F2[x] >: F[x], B](f: A => Free[F2,B]): Free[F2,B] =
FlatMap(this, f)

@scala.annotation.tailrec
final def step: Free[F, A] = this match
case FlatMap(FlatMap(fx, f), g) => fx.flatMap(x => f(x).flatMap(y => g(y))).step
case FlatMap(Return(x), f) => f(x).step
case _ => this
8 changes: 8 additions & 0 deletions tests/pos/i15523.avoid.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// scalac: -Werror
// like the original, but with a case body `a`
// which caused type avoidance to infinitely recurse
sealed trait Parent
final case class Leaf[A, B >: A](a: A, b: B) extends Parent

def run(x: Parent) = x match
case Leaf(a, _) => a
7 changes: 7 additions & 0 deletions tests/pos/i15523.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// scalac: -Werror
sealed trait Parent
final case class Leaf[A, B >: A](a: A, b: B) extends Parent

def run(x: Parent): Unit = x match {
case Leaf(a, b) =>
}
52 changes: 52 additions & 0 deletions tests/pos/i16777.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// scalac: -Ykind-projector:underscores

sealed abstract class Free[+S[_, _], +E, +A] {
@inline final def flatMap[S1[e, a] >: S[e, a], B, E1 >: E](fun: A => Free[S1, E1, B]): Free[S1, E1, B] = Free.FlatMapped[S1, E, E1, A, B](this, fun)
@inline final def map[B](fun: A => B): Free[S, E, B] = flatMap(a => Free.pure[S, B](fun(a)))
@inline final def as[B](as: => B): Free[S, E, B] = map(_ => as)
@inline final def *>[S1[e, a] >: S[e, a], B, E1 >: E](sc: Free[S1, E1, B]): Free[S1, E1, B] = flatMap(_ => sc)
@inline final def <*[S1[e, a] >: S[e, a], B, E1 >: E](sc: Free[S1, E1, B]): Free[S1, E1, A] = flatMap(r => sc.as(r))

@inline final def void: Free[S, E, Unit] = map(_ => ())

// FIXME: Scala 3.1.4 bug: false unexhaustive match warning
/// @nowarn("msg=pattern case: Free.FlatMapped")
@inline final def foldMap[S1[e, a] >: S[e, a], G[+_, +_]](transform: S1 ~>> G)(implicit G: Monad2[G]): G[E, A] = {
this match {
case Free.Pure(a) => G.pure(a)
case Free.Suspend(a) => transform.apply(a)
case Free.FlatMapped(sub, cont) =>
sub match {
case Free.FlatMapped(sub2, cont2) => sub2.flatMap(a => cont2(a).flatMap(cont)).foldMap(transform)
case another => G.flatMap(another.foldMap(transform))(cont(_).foldMap(transform))
}
}
}
}

trait ~>>[-F[_, _], +G[_, _]] {
def apply[E, A](f: F[E, A]): G[E, A]
}

object Free {
@inline def pure[S[_, _], A](a: A): Free[S, Nothing, A] = Pure(a)
@inline def lift[S[_, _], E, A](s: S[E, A]): Free[S, E, A] = Suspend(s)

final case class Pure[S[_, _], A](a: A) extends Free[S, Nothing, A] {
override def toString: String = s"Pure:[$a]"
}
final case class Suspend[S[_, _], E, A](a: S[E, A]) extends Free[S, E, A] {
override def toString: String = s"Suspend:[$a]"
}
final case class FlatMapped[S[_, _], E, E1 >: E, A, B](sub: Free[S, E, A], cont: A => Free[S, E1, B]) extends Free[S, E1, B] {
override def toString: String = s"FlatMapped:[sub=$sub]"
}
}

type Monad2[F[+_, +_]] = Monad3[λ[(`-R`, `+E`, `+A`) => F[E, A]]]

trait Monad3[F[-_, +_, +_]] {
def flatMap[R, E, A, B](r: F[R, E, A])(f: A => F[R, E, B]): F[R, E, B]
def flatten[R, E, A](r: F[R, E, F[R, E, A]]): F[R, E, A] = flatMap(r)(identity)
def pure[A](a: A): F[Any, Nothing, A]
}