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

Attempt to revert 14026 #15343

Merged
merged 4 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,10 @@ object Config {
* reduces the number of allocated denotations by ~50%.
*/
inline val reuseSymDenotations = true

/** If true, check levels of type variables and create fresh ones as needed.
* This is necessary for soundness (see 3ab18a9), but also causes several
* regressions that should be fixed before turning this on.
*/
inline val checkLevels = false
}
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ trait ConstraintHandling {

/** Is `level` <= `maxLevel` or legal in the current context? */
def levelOK(level: Int, maxLevel: Int)(using Context): Boolean =
level <= maxLevel ||
ctx.isAfterTyper || !ctx.typerState.isCommittable || // Leaks in these cases shouldn't break soundness
level == Int.MaxValue // See `nestingLevel` above.
level <= maxLevel
|| ctx.isAfterTyper || !ctx.typerState.isCommittable // Leaks in these cases shouldn't break soundness
|| level == Int.MaxValue // See `nestingLevel` above.
|| !Config.checkLevels

/** If `param` is nested deeper than `maxLevel`, try to instantiate it to a
* fresh type variable of level `maxLevel` and return the new variable.
Expand Down
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import Comments.{Comment, CommentsContext}
import NameKinds._
import StdNames.nme
import transform.SymUtils._
import config.Config
import collection.mutable
import dotty.tools.tasty.TastyFormat.ASTsSection

Expand Down Expand Up @@ -85,6 +86,11 @@ class TreePickler(pickler: TastyPickler) {
case Some(label) =>
if (label != NoAddr) writeRef(label) else pickleForwardSymRef(sym)
case None =>
// See pos/t1957.scala for an example where this can happen.
// I believe it's a bug in typer: the type of an implicit argument refers
// to a closure parameter outside the closure itself. TODO: track this down, so that we
// can eliminate this case.
report.log(i"pickling reference to as yet undefined $sym in ${sym.owner}", sym.srcPos)
pickleForwardSymRef(sym)
}

Expand Down Expand Up @@ -197,7 +203,7 @@ class TreePickler(pickler: TastyPickler) {
}
else if (tpe.prefix == NoPrefix) {
writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
if !symRefs.contains(sym) && !sym.isPatternBound && !sym.hasAnnotation(defn.QuotedRuntimePatterns_patternTypeAnnot) then
if Config.checkLevels && !symRefs.contains(sym) && !sym.isPatternBound && !sym.hasAnnotation(defn.QuotedRuntimePatterns_patternTypeAnnot) then
report.error(i"pickling reference to as yet undefined $tpe with symbol ${sym}", sym.srcPos)
pickleSymRef(sym)
}
Expand Down
18 changes: 17 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1672,7 +1672,23 @@ class Namer { typer: Typer =>
// This case applies if the closure result type contains uninstantiated
// type variables. In this case, constrain the closure result from below
// by the parameter-capture-avoiding type of the body.
typedAheadExpr(mdef.rhs, tpt.tpe).tpe
val rhsType = typedAheadExpr(mdef.rhs, tpt.tpe).tpe

// The following part is important since otherwise we might instantiate
// the closure result type with a plain functon type that refers
// to local parameters. An example where this happens in `dependent-closures.scala`
// If the code after `val rhsType` is commented out, this file fails pickling tests.
// AVOIDANCE TODO: Follow up why this happens, and whether there
// are better ways to achieve this. It would be good if we could get rid of this code.
// It seems at least partially redundant with the nesting level checking on TypeVar
// instantiation.
if !Config.checkLevels then
val hygienicType = TypeOps.avoid(rhsType, termParamss.flatten)
if (!hygienicType.isValueType || !(hygienicType <:< tpt.tpe))
report.error(i"return type ${tpt.tpe} of lambda cannot be made hygienic;\n" +
i"it is not a supertype of the hygienic type $hygienicType", mdef.srcPos)
//println(i"lifting $rhsType over $termParamss -> $hygienicType = ${tpt.tpe}")
//println(TypeComparer.explained { implicit ctx => hygienicType <:< tpt.tpe })
case _ =>
}
WildcardType
Expand Down
3 changes: 3 additions & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,6 @@ i4176-gadt.scala
i13974a.scala

java-inherited-type1

# avoidance bug
i15174.scala
2 changes: 1 addition & 1 deletion tests/neg/i12640.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ case class Empty[F[_]]() extends CpsStream[F,Nothing]

def unfold[S,F[_]:CpsMonad,T](s0:S)(f:S => F[Option[(S,T)]]):F[CpsStream[F,T]] =
summon[CpsMonad[F]].flatMap(f(s0)){
case Some(s1,a) => Cons(a, () => unfold(s1,f)) // error (used to crash)
case Some(s1,a) => Cons(a, () => unfold(s1,f)) // error (used to crash) // error
case None => summon[CpsMonad[F]].pure(Empty[F]())
}
File renamed without changes.
File renamed without changes.
6 changes: 6 additions & 0 deletions tests/pos/i14494.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
object ImplNotFound:
def main(args: Array[String]): Unit =
val res: Seq[String | Int] = (??? : Seq[Int]).collect {
case 1 => Seq("")
case 2 => Seq(1)
}.flatten
17 changes: 17 additions & 0 deletions tests/pos/i15178.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// This should be a neg test once level checking is re-enabled.

trait E[F[_]] {
type T
val value: F[T]
}

object E {
def apply[F[_], T1](value1: F[T1]) = new E[F] {
type T = T1
val value = value1
}
}

val a: Option[E[Ordering]] = Option(E(Ordering[Int]))
val _ = a.map(it => E(it.value)) // there should be an error here

14 changes: 14 additions & 0 deletions tests/pos/i15184.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
def test() = {
func(_ => Box(Seq.empty[String]) )
}

def func[R0](to0: Unit => R0): Unit = ???

trait JsonFormat[T]
object JsonFormat{
implicit def immSeqFormat: JsonFormat[Seq[String]] = ???

implicit def iterableFormat: JsonFormat[Iterable[String]] = ???
}

case class Box[A1: JsonFormat](elem: A1)
25 changes: 25 additions & 0 deletions tests/pos/i15216.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
sealed abstract class Free[S[_], A] {
final def map[B](f: A => B): Free[S, B] = ???
final def flatMap[B](f: A => Free[S, B]): Free[S, B] = new Free[S, B] {}
}

trait Parameter[T]
def namedDouble(name: String): Free[Parameter, Double] = ???

type Double2 = (Double, Double)
type Double3 = (Double, Double, Double)
val spec: Free[Parameter, Either[Double3, Double2]] = for {
result <-
if (???) {
for {
x <- namedDouble("X")
y <- namedDouble("Y")
z <- namedDouble("Z")
} yield Left((x, y, z))
} else {
for {
x <- namedDouble("X")
y <- namedDouble("Y")
} yield Right((x, y))
}
} yield result