Skip to content

Commit

Permalink
Merge pull request #15343 from dotty-staging/revert-14026
Browse files Browse the repository at this point in the history
Attempt to revert 14026
  • Loading branch information
odersky authored Jun 1, 2022
2 parents 89c3c10 + 44c2a3b commit 3ddb21c
Show file tree
Hide file tree
Showing 14 changed files with 100 additions and 6 deletions.
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 @@ -84,3 +84,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.
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

0 comments on commit 3ddb21c

Please sign in to comment.