Merge pull request #5284 from milessabin/topic/si-7046

SI-7046 reflection doesn't see all knownDirectSubclasses

This appears to do the right thing in the most typical scenarios in which `knownDirectSubclasses` would be used. The missing 5% is that subclasses defined in local scopes might not be seen by `knownDirectSubclasses` (see `Local` and `Riddle` in the test below). In mitigation, though, it is almost certain that a local subclass would represent an error in any scenario where `knownDirectSubclasses` might be used.

Errors for such situations are reported by recording (via a symbol attachment) that `knownDirectSubclasses` has been called and reporting an error if any additional children are added subsequently.

Despite these limitations and caveats, I believe that this represents a huge improvement over the status quo, and would eliminate 100% of the failures that I've seen in practice with people using shapeless for type class derivation.

20 files changed, 251 insertions, 14 deletions

diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index 9c9be4eb4d..464fa1ad18 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -185,6 +185,19 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
     }
   }
 
+  private var propCnt = 0
+  @inline final def withPropagateCyclicReferences[T](t: => T): T = {
+    try {
+      propCnt = propCnt+1
+      t
+    } finally {
+      propCnt = propCnt-1
+      assert(propCnt >= 0)
+    }
+  }
+
+  def propagateCyclicReferences: Boolean = propCnt > 0
+
   /** Representing ASTs as graphs */
   object treeBrowsers extends {
     val global: Global.this.type = Global.this
diff --git a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
index fcfcc8feb9..2d8d591b6d 100644
--- a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
@@ -655,9 +655,6 @@ trait ContextErrors {
       def ParentFinalInheritanceError(parent: Tree, mixin: Symbol) =
         NormalTypeError(parent, "illegal inheritance from final "+mixin)
 
-      def ParentSealedInheritanceError(parent: Tree, psym: Symbol) =
-        NormalTypeError(parent, "illegal inheritance from sealed " + psym )
-
       def ParentSelfTypeConformanceError(parent: Tree, selfType: Type) =
         NormalTypeError(parent,
           "illegal inheritance;\n self-type "+selfType+" does not conform to "+
@@ -1172,6 +1169,9 @@ trait ContextErrors {
       def MissingParameterOrValTypeError(vparam: Tree) =
         issueNormalTypeError(vparam, "missing parameter type")
 
+      def ParentSealedInheritanceError(parent: Tree, psym: Symbol) =
+        NormalTypeError(parent, "illegal inheritance from sealed " + psym )
+
       def RootImportError(tree: Tree) =
         issueNormalTypeError(tree, "_root_ cannot be imported")
 
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index 78e8c8c073..7ffc6c6b48 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -115,7 +115,7 @@ trait Namers extends MethodSynthesis {
     protected def owner       = context.owner
     def contextFile = context.unit.source.file
     def typeErrorHandler[T](tree: Tree, alt: T): PartialFunction[Throwable, T] = {
-      case ex: TypeError =>
+      case ex: TypeError if !global.propagateCyclicReferences =>
         // H@ need to ensure that we handle only cyclic references
         TypeSigError(tree, ex)
         alt
@@ -1028,12 +1028,33 @@ trait Namers extends MethodSynthesis {
 
     private def templateSig(templ: Template): Type = {
       val clazz = context.owner
+
+      val parentTrees = typer.typedParentTypes(templ)
+
+      val pending = mutable.ListBuffer[AbsTypeError]()
+      parentTrees foreach { tpt =>
+        val ptpe = tpt.tpe
+        if(!ptpe.isError) {
+          val psym = ptpe.typeSymbol
+          val sameSourceFile = context.unit.source.file == psym.sourceFile
+
+          if (psym.isSealed && !phase.erasedTypes)
+            if (sameSourceFile)
+              psym addChild context.owner
+            else
+              pending += ParentSealedInheritanceError(tpt, psym)
+          if (psym.isLocalToBlock && !phase.erasedTypes)
+            psym addChild context.owner
+        }
+      }
+      pending.foreach(ErrorUtils.issueTypeError)
+
       def checkParent(tpt: Tree): Type = {
         if (tpt.tpe.isError) AnyRefTpe
         else tpt.tpe
       }
 
-      val parents = typer.typedParentTypes(templ) map checkParent
+      val parents = parentTrees map checkParent
 
       enterSelf(templ.self)
 
@@ -1827,6 +1848,12 @@ trait Namers extends MethodSynthesis {
 
   abstract class TypeCompleter extends LazyType {
     val tree: Tree
+    override def forceDirectSuperclasses: Unit = {
+      tree.foreach {
+        case dt: DefTree => global.withPropagateCyclicReferences(Option(dt.symbol).map(_.maybeInitialize))
+        case _ =>
+      }
+    }
   }
 
   def mkTypeCompleter(t: Tree)(c: Symbol => Unit) = new LockingTypeCompleter with FlagAgnosticCompleter {
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index de72d9feed..ec676c30a5 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1678,7 +1678,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           supertpts mapConserve (tpt => checkNoEscaping.privates(context.owner, tpt))
         }
         catch {
-          case ex: TypeError =>
+          case ex: TypeError if !global.propagateCyclicReferences =>
             // fallback in case of cyclic errors
             // @H none of the tests enter here but I couldn't rule it out
             // upd. @E when a definition inherits itself, we end up here
@@ -1739,13 +1739,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             context.deprecationWarning(parent.pos, psym, report, version)
           }
 
-          if (psym.isSealed && !phase.erasedTypes)
-            if (sameSourceFile)
-              psym addChild context.owner
-            else
-              pending += ParentSealedInheritanceError(parent, psym)
-          if (psym.isLocalToBlock && !phase.erasedTypes)
-            psym addChild context.owner
           val parentTypeOfThis = parent.tpe.dealias.typeOfThis
 
           if (!(selfType <:< parentTypeOfThis) &&
@@ -5554,6 +5547,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       }
 
       try runTyper() catch {
+        case ex: CyclicReference if global.propagateCyclicReferences =>
+          throw ex
         case ex: TypeError =>
           tree.clearType()
           // The only problematic case are (recoverable) cyclic reference errors which can pop up almost anywhere.
diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala
index fd8f51cfb1..fc49de1cf6 100644
--- a/src/reflect/scala/reflect/internal/StdAttachments.scala
+++ b/src/reflect/scala/reflect/internal/StdAttachments.scala
@@ -81,4 +81,10 @@ trait StdAttachments {
 
   /** An attachment carrying information between uncurry and erasure */
   case class TypeParamVarargsAttachment(val typeParamRef: Type)
+
+  /** Attached to a class symbol to indicate that its children have been observed
+    * via knownDirectSubclasses. Children added subsequently will trigger an
+    * error to indicate that the earlier observation was incomplete.
+    */
+  case object KnownDirectSubclassesCalled extends PlainAttachment
 }
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 0da153349a..e664b5ad08 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -121,6 +121,16 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def knownDirectSubclasses = {
       // See `getFlag` to learn more about the `isThreadsafe` call in the body of this method.
       if (!isCompilerUniverse && !isThreadsafe(purpose = AllOps)) initialize
+
+      enclosingPackage.info.decls.foreach { sym =>
+        if(sourceFile == sym.sourceFile) {
+          sym.rawInfo.forceDirectSuperclasses
+        }
+      }
+
+      if(!isPastTyper)
+        updateAttachment(KnownDirectSubclassesCalled)
+
       children
     }
 
@@ -3298,7 +3308,12 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
 
     private[this] var childSet: Set[Symbol] = Set()
     override def children = childSet
-    override def addChild(sym: Symbol) { childSet = childSet + sym }
+    override def addChild(sym: Symbol) {
+      if(!isPastTyper && hasAttachment[KnownDirectSubclassesCalled.type] && !childSet.contains(sym))
+        globalError(s"knownDirectSubclasses of ${this.name} observed before subclass ${sym.name} registered")
+
+      childSet = childSet + sym
+    }
 
     def anonOrRefinementString = {
       if (hasCompleteInfo) {
diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index dc35053835..fa5a1a25ad 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -313,6 +313,11 @@ trait Types
     /** If this is a lazy type, assign a new type to `sym`. */
     def complete(sym: Symbol) {}
 
+    /** If this is a lazy type corresponding to a subclass add it to its
+     *  parents children
+     */
+    def forceDirectSuperclasses: Unit = ()
+
     /** The term symbol associated with the type
       * Note that the symbol of the normalized type is returned (@see normalize)
       */
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
index dbafbfc6ba..d5d62b2203 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
@@ -48,6 +48,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.OuterArgCanBeElided
     this.UseInvokeSpecial
     this.TypeParamVarargsAttachment
+    this.KnownDirectSubclassesCalled
     this.noPrint
     this.typeDebug
     this.Range
diff --git a/test/files/neg/t7046-2.check b/test/files/neg/t7046-2.check
new file mode 100644
index 0000000000..b4efd8b5e9
--- /dev/null
+++ b/test/files/neg/t7046-2.check
@@ -0,0 +1,3 @@
+error: knownDirectSubclasses of Foo observed before subclass Bar registered
+error: knownDirectSubclasses of Foo observed before subclass Baz registered
+two errors found
diff --git a/test/files/neg/t7046-2/Macros_1.scala b/test/files/neg/t7046-2/Macros_1.scala
new file mode 100644
index 0000000000..2a5bf82f62
--- /dev/null
+++ b/test/files/neg/t7046-2/Macros_1.scala
@@ -0,0 +1,15 @@
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macros {
+  def impl[T](c: Context)(implicit ttag: c.WeakTypeTag[T]): c.Expr[List[String]] = {
+    import c.universe._;
+    val ttpe = ttag.tpe
+    val tsym = ttpe.typeSymbol.asClass
+    val subclasses = tsym.knownDirectSubclasses.toList.map(_.name.toString)
+
+    c.Expr[List[String]](q"$subclasses")
+  }
+
+  def knownDirectSubclasses[T]: List[String] = macro impl[T]
+}
diff --git a/test/files/neg/t7046-2/Test_2.scala b/test/files/neg/t7046-2/Test_2.scala
new file mode 100644
index 0000000000..18a2ebcbc2
--- /dev/null
+++ b/test/files/neg/t7046-2/Test_2.scala
@@ -0,0 +1,14 @@
+object Test extends App {
+  def nested: Unit = {
+    val subs = Macros.knownDirectSubclasses[Foo]
+    assert(subs == List("Bar", "Baz"))
+
+    sealed trait Foo
+    object Foo {
+      trait Bar extends Foo
+      trait Baz extends Foo
+    }
+  }
+
+  nested
+}
diff --git a/test/files/neg/t7046.check b/test/files/neg/t7046.check
new file mode 100644
index 0000000000..689520a0aa
--- /dev/null
+++ b/test/files/neg/t7046.check
@@ -0,0 +1,3 @@
+error: knownDirectSubclasses of Foo observed before subclass Local registered
+error: knownDirectSubclasses of Foo observed before subclass Riddle registered
+two errors found
diff --git a/test/files/neg/t7046/Macros_1.scala b/test/files/neg/t7046/Macros_1.scala
new file mode 100644
index 0000000000..2a5bf82f62
--- /dev/null
+++ b/test/files/neg/t7046/Macros_1.scala
@@ -0,0 +1,15 @@
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macros {
+  def impl[T](c: Context)(implicit ttag: c.WeakTypeTag[T]): c.Expr[List[String]] = {
+    import c.universe._;
+    val ttpe = ttag.tpe
+    val tsym = ttpe.typeSymbol.asClass
+    val subclasses = tsym.knownDirectSubclasses.toList.map(_.name.toString)
+
+    c.Expr[List[String]](q"$subclasses")
+  }
+
+  def knownDirectSubclasses[T]: List[String] = macro impl[T]
+}
diff --git a/test/files/neg/t7046/Test_2.scala b/test/files/neg/t7046/Test_2.scala
new file mode 100644
index 0000000000..fcb3e46a0f
--- /dev/null
+++ b/test/files/neg/t7046/Test_2.scala
@@ -0,0 +1,35 @@
+object Test extends App {
+  val subs = Macros.knownDirectSubclasses[Foo]
+  assert(subs == List("Wibble", "Wobble", "Bar", "Baz"))
+}
+
+sealed trait Foo
+object Foo {
+  trait Wibble extends Foo
+  case object Wobble extends Foo
+}
+
+trait Bar extends Foo
+
+object Blah {
+  type Quux = Foo
+}
+
+import Blah._
+
+trait Baz extends Quux
+
+class Boz[T](t: T)
+class Unrelated extends Boz(Test.subs)
+
+object Enigma {
+  locally {
+    // local class not seen
+    class Local extends Foo
+  }
+
+  def foo: Unit = {
+    // local class not seen
+    class Riddle extends Foo
+  }
+}
diff --git a/test/files/pos/t7046-2/Macros_1.scala b/test/files/pos/t7046-2/Macros_1.scala
new file mode 100644
index 0000000000..07c0c61281
--- /dev/null
+++ b/test/files/pos/t7046-2/Macros_1.scala
@@ -0,0 +1,14 @@
+package p1
+
+import scala.reflect.macros.blackbox._
+import language.experimental._
+
+object Macro {
+  def impl(c: Context): c.Tree = {
+    import c.universe._
+    val tsym = rootMirror.staticClass("p1.Base")
+    val subclasses = tsym.knownDirectSubclasses.toList.map(_.name.toString)
+    q"$subclasses"
+  }
+  def p1_Base_knownDirectSubclasses: List[String] = macro impl
+}
diff --git a/test/files/pos/t7046-2/Test_2.scala b/test/files/pos/t7046-2/Test_2.scala
new file mode 100644
index 0000000000..74e30a863d
--- /dev/null
+++ b/test/files/pos/t7046-2/Test_2.scala
@@ -0,0 +1,9 @@
+package p1
+
+sealed trait Base
+
+object Test {
+  val x = Macro.p1_Base_knownDirectSubclasses
+}
+
+case class B(val b: Test.x.type)
diff --git a/test/files/run/t7046-1/Macros_1.scala b/test/files/run/t7046-1/Macros_1.scala
new file mode 100644
index 0000000000..2a5bf82f62
--- /dev/null
+++ b/test/files/run/t7046-1/Macros_1.scala
@@ -0,0 +1,15 @@
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macros {
+  def impl[T](c: Context)(implicit ttag: c.WeakTypeTag[T]): c.Expr[List[String]] = {
+    import c.universe._;
+    val ttpe = ttag.tpe
+    val tsym = ttpe.typeSymbol.asClass
+    val subclasses = tsym.knownDirectSubclasses.toList.map(_.name.toString)
+
+    c.Expr[List[String]](q"$subclasses")
+  }
+
+  def knownDirectSubclasses[T]: List[String] = macro impl[T]
+}
diff --git a/test/files/run/t7046-1/Test_2.scala b/test/files/run/t7046-1/Test_2.scala
new file mode 100644
index 0000000000..28459fde72
--- /dev/null
+++ b/test/files/run/t7046-1/Test_2.scala
@@ -0,0 +1,23 @@
+object Test extends App {
+  val subs = Macros.knownDirectSubclasses[Foo]
+  assert(subs == List("Wibble", "Wobble", "Bar", "Baz"))
+}
+
+sealed trait Foo
+object Foo {
+  trait Wibble extends Foo
+  case object Wobble extends Foo
+}
+
+trait Bar extends Foo
+
+object Blah {
+  type Quux = Foo
+}
+
+import Blah._
+
+trait Baz extends Quux
+
+class Boz[T](t: T)
+class Unrelated extends Boz(Test.subs)
diff --git a/test/files/run/t7046-2/Macros_1.scala b/test/files/run/t7046-2/Macros_1.scala
new file mode 100644
index 0000000000..2a5bf82f62
--- /dev/null
+++ b/test/files/run/t7046-2/Macros_1.scala
@@ -0,0 +1,15 @@
+import scala.language.experimental.macros
+import scala.reflect.macros.blackbox.Context
+
+object Macros {
+  def impl[T](c: Context)(implicit ttag: c.WeakTypeTag[T]): c.Expr[List[String]] = {
+    import c.universe._;
+    val ttpe = ttag.tpe
+    val tsym = ttpe.typeSymbol.asClass
+    val subclasses = tsym.knownDirectSubclasses.toList.map(_.name.toString)
+
+    c.Expr[List[String]](q"$subclasses")
+  }
+
+  def knownDirectSubclasses[T]: List[String] = macro impl[T]
+}
diff --git a/test/files/run/t7046-2/Test_2.scala b/test/files/run/t7046-2/Test_2.scala
new file mode 100644
index 0000000000..79407f522f
--- /dev/null
+++ b/test/files/run/t7046-2/Test_2.scala
@@ -0,0 +1,14 @@
+object Test extends App {
+  def nested: Unit = {
+    sealed trait Foo
+    object Foo {
+      trait Bar extends Foo
+      trait Baz extends Foo
+    }
+
+    val subs = Macros.knownDirectSubclasses[Foo]
+    assert(subs == List("Bar", "Baz"))
+  }
+
+  nested
+}