Merge pull request #1014 from dotty-staging/fix/deep-subtypes

Check all bounds and avoid infinite subtyping checks when intersecting denotations

Author: smarter

src/dotty/tools/dotc/core/Denotations.scala

@@ -259,11 +259,12 @@ object Denotations {
      *
      *  If there is no preferred accessible denotation, return a JointRefDenotation
      *  with one of the operand symbols (unspecified which one), and an info which
-     *  is intersection (&) of the infos of the operand denotations.
+     *  is the intersection (using `&` or `safe_&` if `safeIntersection` is true)
+     *  of the infos of the operand denotations.
      *
      *  If SingleDenotations with different signatures are joined, return NoDenotation.
      */
-    def & (that: Denotation, pre: Type)(implicit ctx: Context): Denotation = {
+    def & (that: Denotation, pre: Type, safeIntersection: Boolean = false)(implicit ctx: Context): Denotation = {
 
       /** Try to merge denot1 and denot2 without adding a new signature. */
       def mergeDenot(denot1: Denotation, denot2: SingleDenotation): Denotation = denot1 match {
@@ -317,7 +318,11 @@ object Denotations {
                   else if (preferSym(sym2, sym1)) sym2
                   else sym1
                 val jointInfo =
-                  try info1 & info2
+                  try
+                    if (safeIntersection)
+                      info1 safe_& info2
+                    else
+                      info1 & info2
                   catch {
                     case ex: MergeError =>
                       if (pre.widen.classSymbol.is(Scala2x) || ctx.scala2Mode)

src/dotty/tools/dotc/core/Types.scala

@@ -459,7 +459,7 @@ object Types {
           val jointInfo =
             if (rinfo.isAlias) rinfo
             else if (pdenot.info.isAlias) pdenot.info
-            else if (ctx.pendingMemberSearches.contains(name)) safeAnd(pdenot.info, rinfo)
+            else if (ctx.pendingMemberSearches.contains(name)) pdenot.info safe_& rinfo
             else
               try pdenot.info & rinfo
               catch {
@@ -470,11 +470,15 @@ object Types {
                   // the special shortcut for Any in derivesFrom was as yet absent. To reproduce,
                   // remove the special treatment of Any in derivesFrom and compile
                   // sets.scala.
-                  safeAnd(pdenot.info, rinfo)
+                  pdenot.info safe_& rinfo
               }
           pdenot.asSingleDenotation.derivedSingleDenotation(pdenot.symbol, jointInfo)
-        } else
-          pdenot & (new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId)), pre)
+        } else {
+          pdenot & (
+            new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId)),
+            pre,
+            safeIntersection = ctx.pendingMemberSearches.contains(name))
+        }
       }
       def goThis(tp: ThisType) = {
         val d = go(tp.underlying)
@@ -501,12 +505,10 @@ object Types {
             go(next)
         }
       }
-      def goAnd(l: Type, r: Type) = go(l) & (go(r), pre)
-      def goOr(l: Type, r: Type) = go(l) | (go(r), pre)
-      def safeAnd(tp1: Type, tp2: Type): Type = (tp1, tp2) match {
-        case (TypeBounds(lo1, hi1), TypeBounds(lo2, hi2)) => TypeBounds(lo1 | lo2, AndType(hi1, hi2))
-        case _ => tp1 & tp2
+      def goAnd(l: Type, r: Type) = {
+        go(l) & (go(r), pre, safeIntersection = ctx.pendingMemberSearches.contains(name))
       }
+      def goOr(l: Type, r: Type) = go(l) | (go(r), pre)
 
       { val recCount = ctx.findMemberCount + 1
         ctx.findMemberCount = recCount
@@ -705,6 +707,19 @@ object Types {
       ctx.typeComparer.glb(this, that)
     }
 
+    /** Safer version of `&`.
+     *
+     *  This version does not simplify the upper bound of the intersection of
+     *  two TypeBounds. The simplification done by `&` requires subtyping checks
+     *  which may end up calling `&` again, in most cases this should be safe
+     *  but because of F-bounded types, this can result in an infinite loop
+     *  (which will be masked unless `-Yno-deep-subtypes` is enabled).
+     */
+    def safe_& (that: Type)(implicit ctx: Context): Type = (this, that) match {
+      case (TypeBounds(lo1, hi1), TypeBounds(lo2, hi2)) => TypeBounds(lo1 | lo2, AndType(hi1, hi2))
+      case _ => this & that
+    }
+
     def | (that: Type)(implicit ctx: Context): Type = track("|") {
       ctx.typeComparer.lub(this, that)
     }

src/dotty/tools/dotc/transform/PostTyper.scala

@@ -68,7 +68,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer  { thisTran
     // TODO fill in
   }
 
-  /** Check bounds of AppliedTypeTrees and TypeApplys.
+  /** Check bounds of AppliedTypeTrees.
    *  Replace type trees with TypeTree nodes.
    *  Replace constant expressions with Literal nodes.
    *  Note: Demanding idempotency instead of purity in literalize is strictly speaking too loose.
@@ -97,29 +97,17 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer  { thisTran
    *  Revisit this issue once we have implemented `inline`. Then we can demand
    *  purity of the prefix unless the selection goes to an inline val.
    */
-  private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = {
-    def literalize(tp: Type): Tree = tp.widenTermRefExpr match {
-      case ConstantType(value) if isIdempotentExpr(tree) => Literal(value)
-      case _ => tree
-    }
-    def norm(tree: Tree) =
-      if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos)
-      else literalize(tree.tpe)
-    tree match {
-      case tree: TypeTree =>
-        tree
-      case AppliedTypeTree(tycon, args) =>
-        val tparams = tycon.tpe.typeSymbol.typeParams
-        val bounds = tparams.map(tparam =>
-          tparam.info.asSeenFrom(tycon.tpe.normalizedPrefix, tparam.owner.owner).bounds)
-        Checking.checkBounds(args, bounds, _.substDealias(tparams, _))
-        norm(tree)
-      case TypeApply(fn, args) =>
-        Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType])
-        norm(tree)
-      case _ =>
-        norm(tree)
-    }
+  private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = tree match {
+    case tree: TypeTree => tree
+    case _ =>
+      if (tree.isType) {
+        Checking.boundsChecker.traverse(tree)
+        TypeTree(tree.tpe).withPos(tree.pos)
+      }
+      else tree.tpe.widenTermRefExpr match {
+        case ConstantType(value) if isIdempotentExpr(tree) => Literal(value)
+        case _ => tree
+      }
   }
 
   class PostTyperTransformer extends Transformer {
@@ -161,10 +149,16 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer  { thisTran
           }
         case tree: Select =>
           transformSelect(paramFwd.adaptRef(tree), Nil)
-        case tree @ TypeApply(sel: Select, args) =>
-          val args1 = transform(args)
-          val sel1 = transformSelect(sel, args1)
-          if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree)(sel1, args1)
+        case tree @ TypeApply(fn, args) =>
+          Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType])
+          fn match {
+            case sel: Select =>
+              val args1 = transform(args)
+              val sel1 = transformSelect(sel, args1)
+              if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree)(sel1, args1)
+            case _ =>
+              super.transform(tree)
+          }
         case tree @ Assign(sel: Select, _) =>
           superAcc.transformAssign(super.transform(tree))
         case tree: Template =>
@@ -180,6 +174,16 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer  { thisTran
         case tree: DefDef =>
           transformAnnots(tree)
           superAcc.wrapDefDef(tree)(super.transform(tree).asInstanceOf[DefDef])
+        case tree: TypeDef =>
+          transformAnnots(tree)
+          val sym = tree.symbol
+          val tree1 =
+            if (sym.isClass) tree
+            else {
+              Checking.boundsChecker.traverse(tree.rhs)
+              cpy.TypeDef(tree)(rhs = TypeTree(tree.symbol.info))
+            }
+          super.transform(tree1)
         case tree: MemberDef =>
           transformAnnots(tree)
           super.transform(tree)

src/dotty/tools/dotc/typer/Applications.scala

@@ -607,7 +607,6 @@ trait Applications extends Compatibility { self: Typer =>
       case pt: PolyType =>
         if (typedArgs.length <= pt.paramBounds.length)
           typedArgs = typedArgs.zipWithConserve(pt.paramBounds)(adaptTypeArg)
-        Checking.checkBounds(typedArgs, pt)
       case _ =>
     }
     assignType(cpy.TypeApply(tree)(typedFn, typedArgs), typedFn, typedArgs)

src/dotty/tools/dotc/typer/Checking.scala

@@ -48,6 +48,21 @@ object Checking {
   def checkBounds(args: List[tpd.Tree], poly: PolyType)(implicit ctx: Context): Unit =
     checkBounds(args, poly.paramBounds, _.substParams(poly, _))
 
+  /** Check all AppliedTypeTree nodes in this tree for legal bounds */
+  val boundsChecker = new TreeTraverser {
+    def traverse(tree: Tree)(implicit ctx: Context) = {
+      tree match {
+        case AppliedTypeTree(tycon, args) =>
+          val tparams = tycon.tpe.typeSymbol.typeParams
+          val bounds = tparams.map(tparam =>
+            tparam.info.asSeenFrom(tycon.tpe.normalizedPrefix, tparam.owner.owner).bounds)
+          checkBounds(args, bounds, _.substDealias(tparams, _))
+        case _ =>
+      }
+      traverseChildren(tree)
+    }
+  }
+
   /** Check that `tp` refers to a nonAbstract class
    *  and that the instance conforms to the self type of the created class.
    */

src/dotty/tools/dotc/typer/Namer.scala

@@ -869,7 +869,9 @@ class Namer { typer: Typer =>
       if (tparamSyms.nonEmpty && !isDerived) tp.LambdaAbstract(tparamSyms)
       //else if (toParameterize) tp.parameterizeWith(tparamSyms)
       else tp
-    sym.info = abstracted(TypeBounds.empty)
+
+    val dummyInfo = abstracted(TypeBounds.empty)
+    sym.info = dummyInfo
       // Temporarily set info of defined type T to ` >: Nothing <: Any.
       // This is done to avoid cyclic reference errors for F-bounds.
       // This is subtle: `sym` has now an empty TypeBounds, but is not automatically
@@ -890,6 +892,19 @@ class Namer { typer: Typer =>
       sym.info = NoCompleter
       sym.info = checkNonCyclic(sym, unsafeInfo, reportErrors = true)
     }
+
+    // Here we pay the price for the cavalier setting info to TypeBounds.empty above.
+    // We need to compensate by invalidating caches in references that might
+    // still contain the TypeBounds.empty. If we do not do this, stdlib factories
+    // fail with a bounds error in PostTyper.
+    def ensureUpToDate(tp: Type, outdated: Type) = tp match {
+      case tref: TypeRef if tref.info == outdated && sym.info != outdated =>
+        tref.uncheckedSetSym(null)
+      case _ =>
+    }
+    ensureUpToDate(sym.typeRef, dummyInfo)
+    ensureUpToDate(sym.typeRef.appliedTo(tparamSyms.map(_.typeRef)), TypeBounds.empty)
+
     etaExpandArgs.apply(sym.info)
   }
 

src/dotty/tools/dotc/typer/Typer.scala

@@ -961,8 +961,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
     val TypeDef(name, rhs) = tdef
     checkLowerNotHK(sym, tdef.tparams.map(symbolOfTree), tdef.pos)
     completeAnnotations(tdef, sym)
-    val _ = typedType(rhs) // unused, typecheck only to remove from typedTree
-    assignType(cpy.TypeDef(tdef)(name, TypeTree(sym.info), Nil), sym)
+    assignType(cpy.TypeDef(tdef)(name, typedType(rhs), Nil), sym)
   }
 
   def typedClassDef(cdef: untpd.TypeDef, cls: ClassSymbol)(implicit ctx: Context) = track("typedClassDef") {

test/dotc/tests.scala

@@ -108,8 +108,8 @@ class tests extends CompilerTest {
 
   @Test def neg_abstractOverride() = compileFile(negDir, "abstract-override", xerrors = 2)
   @Test def neg_blockescapes() = compileFile(negDir, "blockescapesNeg", xerrors = 1)
-  @Test def neg_bounds() = compileFile(negDir, "bounds", xerrors = 2)
-  @Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 4)
+  @Test def neg_bounds() = compileFile(negDir, "bounds", xerrors = 3)
+  @Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 3)
   @Test def neg_typedIdents() = compileDir(negDir, "typedIdents", xerrors = 2)
   @Test def neg_assignments() = compileFile(negDir, "assignments", xerrors = 3)
   @Test def neg_typers() = compileFile(negDir, "typers", xerrors = 14)(allowDoubleBindings)
@@ -167,6 +167,7 @@ class tests extends CompilerTest {
   @Test def neg_selfreq = compileFile(negDir, "selfreq", xerrors = 2)
   @Test def neg_singletons = compileFile(negDir, "singletons", xerrors = 8)
   @Test def neg_shadowedImplicits = compileFile(negDir, "arrayclone-new", xerrors = 2)
+  @Test def neg_ski = compileFile(negDir, "ski", xerrors = 2)
   @Test def neg_traitParamsTyper = compileFile(negDir, "traitParamsTyper", xerrors = 5)
   @Test def neg_traitParamsMixin = compileFile(negDir, "traitParamsMixin", xerrors = 2)
   @Test def neg_firstError = compileFile(negDir, "firstError", xerrors = 3)
@@ -175,6 +176,7 @@ class tests extends CompilerTest {
   @Test def neg_validateParsing = compileFile(negDir, "validate-parsing", xerrors = 7)
   @Test def neg_validateRefchecks = compileFile(negDir, "validate-refchecks", xerrors = 2)
   @Test def neg_skolemize = compileFile(negDir, "skolemize", xerrors = 2)
+  @Test def neg_nested_bounds = compileFile(negDir, "nested_bounds", xerrors = 1)
 
   @Test def run_all = runFiles(runDir)
 

tests/neg/bounds.scala

@@ -5,4 +5,8 @@ object Test {
     g("foo")
     new C("bar")
   }
+  def baz[X >: Y, Y <: String](x: X, y: Y) = (x, y)
+
+  baz[Int, String](1, "abc")
+
 }

tests/neg/nested_bounds.scala

@@ -0,0 +1,5 @@
+class OnlyInt[T <: Int]
+
+object Test {
+  type T = List[OnlyInt[String]] // error: Type argument String does not conform to upper bound Int
+}