Polishing of Denotations and Transformers.

3 files changed, 137 insertions, 69 deletions

diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala
index 314e19533..3e6367f2a 100644
--- a/src/dotty/tools/dotc/core/Denotations.scala
+++ b/src/dotty/tools/dotc/core/Denotations.scala
@@ -29,11 +29,9 @@ import Decorators.SymbolIteratorDecorator
  *    |                    |                       |                |
  *    | denot              | denot                 | denot          | denot
  *    v                    v                       v                v
- *  Denotation-+-----SingleDenotation-+------SymDenotation-+----ClassDenotation-+--CompleteClassDenotation
- *             |                      |                    |                    +--LazyClassDenotation
- *             +-----MultiDenotation  |                    |
- *                                    |                    +--CompleteSymDenotation
- *                                    |                    +--LazySymDenotation
+ *  Denotation-+-----SingleDenotation-+------SymDenotation-+----ClassDenotation
+ *             |                      |
+ *             +-----MultiDenotation  |
  *                                    |
  *                                    +--UniqueRefDenotation
  *                                    +--JointRefDenotation
@@ -60,14 +58,7 @@ import Decorators.SymbolIteratorDecorator
  *                              flags: Flags
  *                              privateWithin: Symbol
  *                              annotations: List[Annotation]
- *  ClassDenotation          A denotation representing a single class definition, with new fields
- *                              typeParams: List[TypeSymbol]
- *                              parents: List[Type]
- *                              decls: Scope
- *  LazySymDenotation        A sym-denotation with fields that are computed on demand
- *  CompleteSymDenotation    A sym-denotation that has all fields completed
- *  LazyClassDenotation      A class denotation with fields that are computed on demand
- *  CompleteClassDenotation  A class denotation that has all fields completed
+ *  ClassDenotation          A denotation representing a single class definition.
  */
 object Denotations {
 
@@ -115,7 +106,7 @@ object Denotations {
    *
    *    class A { def f: A }
    *    class B { def f: B }
-   *    val x: A | B = if (???) new A else new B
+   *    val x: A | B = if (test) new A else new B
    *    val y = x.f
    *
    *  Then the denotation of `y` is `SingleDenotation(NoSymbol, A | B)`.
@@ -149,12 +140,15 @@ object Denotations {
     /** The variant of this denotation that's current in the given context. */
     def current(implicit ctx: Context): Denotation
 
-    /** The first non-empty symbol, exists also for overloaded denotations,
-     *  where an arbitrary variant's symbol is picked (with preference for existsing symbols) */
-    def firstSym(implicit ctx: Context): Symbol
-
+    /** Does this denotation exist?
+     *  A denotation does not exist if it has NoType as an info.
+     *  This is the case for NoDenotation, and also for SymDenotations
+     *  that come from package members (classes or modules) where no
+     *  corresponding symbol was found in a classfile or source.
+     */
     def exists: Boolean = true
 
+    /** If this denotation does not exist, fallback to alternative */
     def orElse(that: => Denotation) = if (this.exists) this else that
 
     /** The set of alternative single-denotations making up this denotation */
@@ -191,20 +185,21 @@ object Denotations {
     def requiredSymbol(p: Symbol => Boolean, source: AbstractFile = null)(implicit ctx: Context): Symbol = {
       val sym = disambiguate(p).symbol
       if (sym.exists) sym else {
+        val firstSym = ((NoSymbol: Symbol) /: alternatives.map(_.symbol)) (_ orElse _)
         val owner = if (firstSym.exists) firstSym.owner else NoSymbol
         ctx.newStubSymbol(owner, firstSym.name, source)
       }
     }
 
     /** Form a denotation by conjoining with denotation `that` */
-    def &(that: Denotation)(implicit ctx: Context): Denotation =
+    def & (that: Denotation)(implicit ctx: Context): Denotation =
       if (this eq that) this
       else if (!this.exists) that
       else if (!that.exists) this
       else that match {
         case that: SingleDenotation =>
           val r = mergeDenot(this, that)
-          if (r ne NoDenotation) r else MultiDenotation(this, that)
+          if (r.exists) r else MultiDenotation(this, that)
         case that @ MultiDenotation(denot1, denot2) =>
           this & denot1 & denot2
       }
@@ -215,13 +210,24 @@ object Denotations {
     private def mergeDenot(denot1: Denotation, denot2: SingleDenotation)(implicit ctx: Context): Denotation = denot1 match {
       case denot1 @ MultiDenotation(denot11, denot12) =>
         val d1 = mergeDenot(denot11, denot2)
-        if (d1 ne NoDenotation) d1 else mergeDenot(denot12, denot2)
+        if (d1.exists) denot1.derivedMultiDenotation(d1, denot2)
+        else {
+          val d2 = mergeDenot(denot12, denot2)
+          if (d2.exists) denot1.derivedMultiDenotation(denot11, d2)
+          else NoDenotation
+        }
       case denot1: SingleDenotation =>
         if (denot1 eq denot2) denot1
         else if (denot1.signature == denot2.signature) {
+          /** symbols eligible for a joint denotation are:
+           *   - type symbols, as long as they are not classes
+           *   - term symbols, where concrete symbols take precedence over abstract ones.
+           */
           def isEligible(sym1: Symbol, sym2: Symbol) =
             if (sym1.isType) !sym1.isClass
-            else !(sym1 is Deferred) || (sym2 is Deferred) || !sym2.exists
+            else sym1.exists && (
+              !(sym1 is Deferred) || !sym2.exists || (sym2 is Deferred))
+          /** Convert class info C to bounds C..C */
           def normalize(info: Type) =
             if (isType) info.bounds else info
           val sym1 = denot1.symbol
@@ -241,7 +247,11 @@ object Denotations {
         } else NoDenotation
     }
 
-    def |(that: Denotation)(pre: Type)(implicit ctx: Context): Denotation = {
+    /** Form a choice between this denotation and that one.
+     *  @param pre  The prefix type of the members of the denotation, used
+     *              to determine an accessible symbol if it exists.
+     */
+    def | (that: Denotation)(pre: Type)(implicit ctx: Context): Denotation = {
 
       def lubSym(sym1: Symbol, sym2: Symbol): Symbol = {
         def qualifies(sym: Symbol) =
@@ -281,17 +291,18 @@ object Denotations {
     def show(implicit ctx: Context): String = ctx.show(this)
   }
 
-  /** The class of overloaded denotations
-   *  @param  variants   The overloaded variants indexed by thheir signatures.
+  /** An overloaded denotation consisting of the alternatives of both given denotations.
    */
   case class MultiDenotation(denot1: Denotation, denot2: Denotation) extends Denotation {
-    final override def isType = false
-    def derivedMultiDenotation(d1: Denotation, d2: Denotation) =
-      if ((d1 eq denot1) && (d2 eq denot2)) this else MultiDenotation(d1, d2)
-    def symbol = unsupported("symbol")
-    def info = unsupported("info")
+    final def symbol = unsupported("symbol")
+    final def info = unsupported("info")
+    final def validFor = denot1.validFor & denot2.validFor
+    final def isType = false
     def signature(implicit ctx: Context) = unsupported("signature")
-    def firstSym(implicit ctx: Context): Symbol = denot1.firstSym orElse denot2.firstSym
+    def atSignature(sig: Signature)(implicit ctx: Context): SingleDenotation =
+      denot1.atSignature(sig) orElse denot2.atSignature(sig)
+    def current(implicit ctx: Context): Denotation =
+      derivedMultiDenotation(denot1.current, denot2.current)
     def altsWith(p: Symbol => Boolean)(implicit ctx: Context): List[SingleDenotation] =
       denot1.altsWith(p) ++ denot2.altsWith(p)
     def suchThat(p: Symbol => Boolean)(implicit ctx: Context): SingleDenotation = {
@@ -304,17 +315,16 @@ object Denotations {
     }
     def hasAltWith(p: Symbol => Boolean)(implicit ctx: Context): Boolean =
       denot1.hasAltWith(p) || denot2.hasAltWith(p)
-    def atSignature(sig: Signature)(implicit ctx: Context): SingleDenotation =
-      denot1.atSignature(sig) orElse denot2.atSignature(sig)
-    def validFor = denot1.validFor & denot2.validFor
-    def current(implicit ctx: Context): Denotation =
-      derivedMultiDenotation(denot1.current, denot2.current)
+    def derivedMultiDenotation(d1: Denotation, d2: Denotation) =
+      if ((d1 eq denot1) && (d2 eq denot2)) this else MultiDenotation(d1, d2)
   }
 
+  /** A non-overloaded denotation */
   abstract class SingleDenotation extends Denotation with PreDenotation {
     override def isType = info.isInstanceOf[TypeType]
     override def signature(implicit ctx: Context): Signature = {
-      def sig(tp: Type): Signature = tp match {
+      if (isType) NotAMethod
+      else info match {
         case tp: PolyType =>
           tp.resultType match {
             case mt: MethodType => mt.signature
@@ -323,14 +333,13 @@ object Denotations {
         case mt: MethodType => mt.signature
         case _ => NotAMethod
       }
-      if (isType) NotAMethod else sig(info)
     }
-    def firstSym(implicit ctx: Context): Symbol = symbol
 
-    def derivedSingleDenotation(s: Symbol, i: Type): SingleDenotation =
-      if ((s eq symbol) && (i eq info)) this else newLikeThis(s, i)
+    def derivedSingleDenotation(symbol: Symbol, info: Type): SingleDenotation =
+      if ((symbol eq this.symbol) && (info eq this.info)) this
+      else newLikeThis(symbol, info)
 
-    protected def newLikeThis(s: Symbol, i: Type): SingleDenotation = this
+    protected def newLikeThis(symbol: Symbol, info: Type): SingleDenotation = this
 
     def orElse(that: => SingleDenotation) = if (this.exists) this else that
 
@@ -362,11 +371,11 @@ object Denotations {
      *  @See current. Flock members are connected in a ring
      *  with their `nextInRun` fields.
      *
-     *  There are the following invariants converning flock members
+     *  There are the following invariants concerning flock members
      *
-     *  1) validity periods must be non-overlapping
-     *  2) the union of all validity periods must be a contiguous
-     *     interval starting in FirstPhaseId.
+     *  1) validity periods are non-overlapping
+     *  2) the union of all validity periods is a contiguous
+     *     interval.
      */
     var nextInRun: SingleDenotation = this
 
@@ -379,9 +388,16 @@ object Denotations {
       current
     }
 
+    /** Produce a denotation in the same flock that is valid for given context
+     *  If one doesn't already exist, create it.
+     *  Pre: validFor.runId == ctx.period.runId.
+     *  Usually called when !(validFor contains ctx.period)
+     *  (even though this is not a precondition).
+     */
     def current(implicit ctx: Context): SingleDenotation = {
       val currentPeriod = ctx.period
       val valid = _validFor
+      assert(valid.runId == currentPeriod.runId)
       var current = this
       if (currentPeriod.code > valid.code) {
         // search for containing period as long as nextInRun increases.
@@ -392,26 +408,31 @@ object Denotations {
           next = next.nextInRun
         }
         if (next.validFor.code > valid.code) {
-          // in this case, containsPeriod(next._validFor, currentPeriod)
+          // in this case, next.validFor contains currentPeriod
           current = next
         } else {
           // not found, current points to highest existing variant
           var startPid = current.validFor.lastPhaseId + 1
-          val trans = ctx.transformersFor(current)
-          val endPid = trans.nextTransformer(startPid + 1).phaseId - 1
-          next = trans.nextTransformer(startPid) transform current
+          val transformers = ctx.transformersFor(current)
+          val transformer = transformers.nextTransformer(startPid)
+          next = transformer transform current
           if (next eq current)
             startPid = current.validFor.firstPhaseId
           else {
             current.nextInRun = next
             current = next
           }
-          current.validFor = Period(currentPeriod.runId, startPid, endPid)
+          current.validFor = Period(
+              currentPeriod.runId, startPid, transformer.lastPhaseId)
         }
       } else {
         // currentPeriod < valid; in this case a version must exist
+        // but to be defensive we check for infinite loop anyway
+        var cnt = 0
         do {
           current = current.nextInRun
+          cnt += 1
+          assert(cnt <= MaxPossiblePhaseId)
         } while (!(current.validFor contains currentPeriod))
       }
       current
@@ -457,18 +478,44 @@ object Denotations {
 
   // --------------- PreDenotations -------------------------------------------------
 
-  /** A PreDenotation represents a set of single denotations
+  /** A PreDenotation represents a group of single denotations
    *  It is used as an optimization to avoid forming MultiDenotations too eagerly.
    */
   trait PreDenotation {
+
+    /** A denotation in the group exists */
     def exists: Boolean
+
+    /** First denotation in the group */
     def first: Denotation
+
+    /** Convert to full denotation by &-ing all elements */
     def toDenot(implicit ctx: Context): Denotation
+
+    /** Group contains a denotation with given signature */
     def containsSig(sig: Signature)(implicit ctx: Context): Boolean
+
+    /** Keep only those denotations in this group which have a signature
+     *  that's not already defined by `denots`.
+     */
     def filterDisjoint(denots: PreDenotation)(implicit ctx: Context): PreDenotation
+
+    /** Keep only those denotations in this group whose flags do not intersect
+     *  with given `flags`.
+     */
     def filterExcluded(flags: FlagSet)(implicit ctx: Context): PreDenotation
+
+    /** Keep only those denotations in this group which are accessible from
+     *  type `pre`.
+     */
     def filterAccessibleFrom(pre: Type)(implicit ctx: Context): PreDenotation
+
+    /** The denotations as seen from given prefix type `pre`, where `owner`
+     *  is assumed to be the owner of all denotation symbols.
+     */
     def asSeenFrom(pre: Type, owner: Symbol)(implicit ctx: Context): PreDenotation
+
+    /** The union of two groups. */
     def union(that: PreDenotation) =
       if (!this.exists) that
       else if (that.exists) this
@@ -477,11 +524,6 @@ object Denotations {
 
   case class DenotUnion(denots1: PreDenotation, denots2: PreDenotation) extends PreDenotation {
     assert(denots1.exists && denots2.exists)
-    private def derivedUnion(s1: PreDenotation, s2: PreDenotation) =
-      if (!s1.exists) s2
-      else if (!s2.exists) s1
-      else if ((s1 eq denots2) && (s2 eq denots2)) this
-      else new DenotUnion(s1, s2)
     def exists = true
     def first = denots1.first
     def toDenot(implicit ctx: Context) = denots1.toDenot & denots2.toDenot
@@ -497,10 +539,16 @@ object Denotations {
       derivedUnion(denots1 filterAccessibleFrom pre, denots2 filterAccessibleFrom pre)
     def asSeenFrom(pre: Type, owner: Symbol)(implicit ctx: Context): PreDenotation =
       derivedUnion(denots1.asSeenFrom(pre, owner), denots2.asSeenFrom(pre, owner))
+    private def derivedUnion(denots1: PreDenotation, denots2: PreDenotation) =
+      if ((denots1 eq this.denots1) && (denots2 eq this.denots2)) this
+      else denots1 union denots2
   }
 
-  /** Creation method for denotations */
+  // --------------- Context Base Trait -------------------------------
+
   trait DenotationsBase { this: ContextBase =>
+
+    /** The current denotation of the static reference given by path. */
     def staticRef(path: Name)(implicit ctx: Context): Denotation = {
       def recur(path: Name, len: Int): Denotation = {
         val point = path.lastIndexOf('.', len - 1)
@@ -508,7 +556,7 @@ object Denotations {
           if (point > 0) recur(path.toTermName, point).disambiguate(_.isParameterless)
           else if (path.isTermName) defn.RootClass.denot
           else defn.EmptyPackageClass.denot
-        if (!owner.exists) owner
+        if (!owner.exists) NoDenotation
         else {
           val name = path slice (point + 1, len)
           val result = owner.info.member(name)
@@ -523,6 +571,10 @@ object Denotations {
       recur(path, path.length)
     }
 
+    /** If we are looking for a non-existing term name in a package,
+     *  assume it is a package for which we do not have a directory and
+     *  enter it.
+     */
     def missingHook(owner: Symbol, name: Name)(implicit ctx: Context): Symbol =
       if (owner.isPackage && name.isTermName)
         ctx.newCompletePackageSymbol(owner, name.asTermName).entered
diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala
index 799407b0c..3ce012968 100644
--- a/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -258,8 +258,7 @@ object SymDenotations {
     def isSourceMethod = this is (Method, butNot = Accessor)
 
     /** Is this NOT a user-defined "def" method that takes parameters? */
-    def isParameterless(implicit ctx: Context) =
-      !isSourceMethod || info.paramTypess.isEmpty
+    def isParameterless(implicit ctx: Context) = signature == NotAMethod
 
     /** Is this a setter? */
     def isGetter = (this is Accessor) && !originalName.isSetterName
diff --git a/src/dotty/tools/dotc/core/Transformers.scala b/src/dotty/tools/dotc/core/Transformers.scala
index 1244ddfac..ba26c8be8 100644
--- a/src/dotty/tools/dotc/core/Transformers.scala
+++ b/src/dotty/tools/dotc/core/Transformers.scala
@@ -11,37 +11,54 @@ object Transformers {
   trait TransformerBase { self: ContextBase =>
 
     def transformersFor(ref: SingleDenotation): TransformerGroup = ref match {
-      case _: SymDenotation => denotTransformers
+      case _: SymDenotation => symTransformers
       case _ => refTransformers
     }
 
-    val denotTransformers = new TransformerGroup
+    val symTransformers = new TransformerGroup
     val refTransformers = new TransformerGroup
   }
 
-  val lastPhaseId = 31
-
+  /** A transformer group contains a sequence of transformers,
+   *  ordered by the phase where they apply. Transformers are added
+   *  to a group via `install`.
+   *
+   *  There are two transformerGroups in a context base:
+   *  symTransformers and refTransformers. symTransformers translate
+   *  full symbol denotations, refTransformers translate only symbol references
+   *  of type Unique/JointRefDenotation.
+   */
   class TransformerGroup {
 
+    /** A transformer transforms denotations at a given phase */
     abstract class Transformer extends DotClass {
+
+      /** The phase at the start of which the denotations are transformed */
       val phaseId: Int
+
+      /** The last phase during which the transformed denotations are valid */
       def lastPhaseId = nextTransformer(phaseId).phaseId - 1
+
+      /** The validity period of the transformer in the given context */
       def validFor(implicit ctx: Context): Period =
         Period(ctx.runId, phaseId, lastPhaseId)
+
+      /** The transformation method */
       def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation
     }
 
+    /** A sentinel transformer object */
     object NoTransformer extends Transformer {
-      val phaseId = Transformers.lastPhaseId + 1
-      override def lastPhaseId = phaseId - 1 // TODO JZ Probably off-by-N error here.
+      val phaseId = MaxPossiblePhaseId + 1
+      override def lastPhaseId = phaseId - 1 // TODO JZ Probably off-by-N error here. MO: Don't think so: we want empty validity period.
       def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation =
         unsupported("transform")
     }
 
     private val nxTransformer =
-      Array.fill[Transformer](lastPhaseId + 1)(NoTransformer)
+      Array.fill[Transformer](MaxPossiblePhaseId + 1)(NoTransformer)
 
-    def nextTransformer(i: Int) = nxTransformer(i)
+    def nextTransformer(pid: PhaseId) = nxTransformer(pid)
 
     def install(pid: PhaseId, trans: Transformer): Unit =
       if ((pid > NoPhaseId) && (nxTransformer(pid).phaseId > pid)) {