Fix/overriding

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

@@ -241,8 +241,7 @@ object Denotations {
             val sym2 = denot2.symbol
             val sym2Accessible = sym2.isAccessibleFrom(pre)
             def prefer(info1: Type, sym1: Symbol, info2: Type, sym2: Symbol) =
-              info1 <:< info2 &&
-              (sym1.isAsConcrete(sym2) || !(info2 <:< info1))
+              info1.overrides(info2) && (sym1.isAsConcrete(sym2) || !info2.overrides(info1))
             if (sym2Accessible && prefer(info2, sym2, info1, sym1)) denot2
             else {
               val sym1Accessible = sym1.isAccessibleFrom(pre)

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

@@ -805,7 +805,13 @@ object SymDenotations {
      *  either as overrider or overridee.
      */
     final def canMatchInheritedSymbols(implicit ctx: Context): Boolean =
-      maybeOwner.isClass && !isConstructor && !is(Private)
+      maybeOwner.isClass && memberCanMatchInheritedSymbols
+
+    /** If false, this class member cannot possibly participate in an override,
+     *  either as overrider or overridee.
+     */
+    final def memberCanMatchInheritedSymbols(implicit ctx: Context): Boolean =
+      !isConstructor && !is(Private)
 
     /** The symbol, in class `inClass`, that is overridden by this denotation. */
     final def overriddenSymbol(inClass: ClassSymbol)(implicit ctx: Context): Symbol =

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

@@ -197,8 +197,6 @@ trait TypeOps { this: Context =>
     }
   }
 
-
-
   private def enterArgBinding(formal: Symbol, info: Type, cls: ClassSymbol, decls: Scope) = {
     val lazyInfo = new LazyType { // needed so we do not force `formal`.
       def complete(denot: SymDenotation)(implicit ctx: Context): Unit = {
@@ -207,7 +205,11 @@ trait TypeOps { this: Context =>
       }
     }
     val typeArgFlag = if (formal is Local) TypeArgument else EmptyFlags
-    val sym = ctx.newSymbol(cls, formal.name, formal.flagsUNSAFE & RetainedTypeArgFlags | typeArgFlag, lazyInfo)
+    val sym = ctx.newSymbol(
+      cls, formal.name,
+      formal.flagsUNSAFE & RetainedTypeArgFlags | typeArgFlag | Override,
+      lazyInfo,
+      coord = cls.coord)
     cls.enter(sym, decls)
   }
 

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

@@ -533,6 +533,22 @@ object Types {
       ctx.typeComparer.isSameType(this, that)
     }
 
+    /** Is this type a legal type for a member that overrides another
+     *  member of type `that`? This is the same as `<:<`, except that
+     *  the types ()T and => T are identified, and T is seen as overriding
+     *  either type.
+     */
+    final def overrides(that: Type)(implicit ctx: Context) = {
+      def result(tp: Type): Type = tp match {
+        case ExprType(_) | MethodType(Nil, _) => tp.resultType
+        case _ => tp
+      }
+      this <:< that || {
+        val rthat = result(that)
+        (rthat ne that) && result(this) <:< rthat
+      }
+    }
+
     /** Is this type close enough to that type so that members
      *  with the two type would override each other?d
      *  This means:

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

@@ -2,9 +2,9 @@ package dotty.tools.dotc
 package transform
 
 import core._
-import Flags._, Symbols._, Contexts._, Types._, Scopes._
+import Flags._, Symbols._, Contexts._, Types._, Scopes._, Decorators._
 import util.HashSet
-import collection.mutable.HashMap
+import collection.mutable
 import collection.immutable.BitSet
 import scala.annotation.tailrec
 
@@ -28,8 +28,7 @@ object OverridingPairs {
     /** Symbols to exclude: Here these are constructors and private locals.
      *  But it may be refined in subclasses.
      */
-    protected def exclude(sym: Symbol): Boolean =
-      sym.isConstructor || sym.is(PrivateLocal)
+    protected def exclude(sym: Symbol): Boolean = !sym.memberCanMatchInheritedSymbols
 
     /** The parents of base (may also be refined).
      */
@@ -70,20 +69,20 @@ object OverridingPairs {
     }
 
     private val subParents = {
-      val subParents = new HashMap[Symbol, BitSet]
+      val subParents = new mutable.HashMap[Symbol, BitSet]
       for (bc <- base.info.baseClasses)
         subParents(bc) = BitSet(parents.indices.filter(parents(_).derivesFrom(bc)): _*)
       subParents
     }
 
     private def hasCommonParentAsSubclass(cls1: Symbol, cls2: Symbol): Boolean =
-      (subParents(cls1) intersect subParents(cls2)).isEmpty
+      (subParents(cls1) intersect subParents(cls2)).nonEmpty
 
     /** The scope entries that have already been visited as overridden
      *  (maybe excluded because of hasCommonParentAsSubclass).
      *  These will not appear as overriding
      */
-    private val visited = new HashSet[ScopeEntry](64)
+    private val visited = new mutable.HashSet[Symbol]
 
     /** The current entry candidate for overriding
      */
@@ -99,48 +98,48 @@ object OverridingPairs {
     var overridden: Symbol = _
 
     //@M: note that next is called once during object initialization
-    def hasNext: Boolean = curEntry ne null
+    final def hasNext: Boolean = nextEntry ne null
 
-    @tailrec
-    final def next(): Unit = {
-      if (curEntry ne null) {
-        overriding = curEntry.sym
+    /**  @post
+     *     curEntry   = the next candidate that may override something else
+     *     nextEntry  = curEntry
+     *     overriding = curEntry.sym
+     */
+    private def nextOverriding(): Unit = {
+      @tailrec def loop(): Unit =
+        if (curEntry ne null) {
+          overriding = curEntry.sym
+          if (visited.contains(overriding)) {
+            curEntry = curEntry.prev
+            loop()
+          }
+        }
+      loop()
+      nextEntry = curEntry
+    }
+
+    /** @post
+     *    hasNext    = there is another overriding pair
+     *    overriding = overriding member of the pair, provided hasNext is true
+     *    overridden = overridden member of the pair, provided hasNext is true
+     */
+    @tailrec final def next(): Unit =
+      if (nextEntry ne null) {
+        nextEntry = decls.lookupNextEntry(nextEntry)
         if (nextEntry ne null) {
-          val overridingOwner = overriding.owner
-          do {
-            do {
-              nextEntry = decls.lookupNextEntry(nextEntry);
-              /* DEBUG
-              if ((nextEntry ne null) &&
-                  !(nextEntry.sym hasFlag PRIVATE) &&
-                  !(overriding.owner == nextEntry.sym.owner) &&
-                  !matches(overriding, nextEntry.sym))
-                println("skipping "+overriding+":"+self.memberType(overriding)+overriding.locationString+" to "+nextEntry.sym+":"+self.memberType(nextEntry.sym)+nextEntry.sym.locationString)
-              */
-              } while ((nextEntry ne null) &&
-                       (//!!!!nextEntry.sym.canMatchInheritedSymbols ||
-                        (overriding.owner == nextEntry.sym.owner) ||
-                        (!matches(overriding, nextEntry.sym)) ||
-                        (exclude(overriding))))
-            if (nextEntry ne null) visited.addEntry(nextEntry)
-            // skip nextEntry if a class in `parents` is a subclass of the owners of both
-            // overriding and nextEntry.sym
-          } while ((nextEntry ne null) &&
-                   hasCommonParentAsSubclass(overridingOwner, nextEntry.sym.owner))
-          if (nextEntry ne null) {
-            overridden = nextEntry.sym;
-            //Console.println("yield: " + overriding + overriding.locationString + " / " + overridden + overridden.locationString);//DEBUG
-          } else {
-            do {
-              curEntry = curEntry.prev
-            } while ((curEntry ne null) && visited.contains(curEntry))
-            nextEntry = curEntry
-            next
+          overridden = nextEntry.sym
+          if (overriding.owner != overridden.owner && matches(overriding, overridden)) {
+            visited += overridden
+            if (!hasCommonParentAsSubclass(overriding.owner, overridden.owner)) return
           }
+        } else {
+          curEntry = curEntry.prev
+          nextOverriding()
         }
+        next()
       }
-    }
 
-    next
+    nextOverriding()
+    next()
   }
 }

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

@@ -1376,8 +1376,8 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
 
       def subPatTypes: List[Type] = typedPatterns map (_.tpe)
 
-      // there are `productArity` non-seq elements in the tuple.
-      protected def firstIndexingBinder = productArity
+      // there are `prodArity` non-seq elements in the tuple.
+      protected def firstIndexingBinder = prodArity
       protected def expectedLength      = elementArity
       protected def lastIndexingBinder  = totalArity - starArity - 1
 
@@ -1535,7 +1535,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
       // the trees that select the subpatterns on the extractor's result, referenced by `binder`
       // require (totalArity > 0 && (!lastIsStar || isSeq))
       protected def subPatRefs(binder: Symbol, subpatBinders: List[Symbol], binderTypeTested: Type): List[Tree] = {
-        if(aligner.isSingle && aligner.extractor.productArity == 1 && defn.isTupleType(binder.info)) {
+        if(aligner.isSingle && aligner.extractor.prodArity == 1 && defn.isTupleType(binder.info)) {
           // special case for extractor
           // comparing with scalac additional assertions added
           val subpw = subpatBinders.head.info.widen
@@ -1577,13 +1577,13 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
     *
     *  Here Pm/Fi is the last pattern to match the fixed arity section.
     *
-    *    productArity: the value of i, i.e. the number of non-sequence types in the extractor
+    *    prodArity: the value of i, i.e. the number of non-sequence types in the extractor
     *    nonStarArity: the value of j, i.e. the number of non-star patterns in the case definition
     *    elementArity: j - i, i.e. the number of non-star patterns which must match sequence elements
     *       starArity: 1 or 0 based on whether there is a star (sequence-absorbing) pattern
     *      totalArity: nonStarArity + starArity, i.e. the number of patterns in the case definition
     *
-    *  Note that productArity is a function only of the extractor, and
+    *  Note that prodArity is a function only of the extractor, and
     *  nonStar/star/totalArity are all functions of the patterns. The key
     *  value for aligning and typing the patterns is elementArity, as it
     *  is derived from both sets of information.
@@ -1651,7 +1651,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
     final case class Extractor(whole: Type, fixed: List[Type], repeated: Repeated) {
       require(whole != NoType, s"expandTypes($whole, $fixed, $repeated)")
 
-      def productArity = fixed.length
+      def prodArity    = fixed.length
       def hasSeq       = repeated.exists
       def elementType  = repeated.elementType
       def sequenceType = repeated.sequenceType
@@ -1681,8 +1681,8 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
       *  < 0: There are more products than patterns: compile time error.
       */
     final case class Aligned(patterns: Patterns, extractor: Extractor) {
-      def elementArity = patterns.nonStarArity - productArity
-      def productArity = extractor.productArity
+      def elementArity = patterns.nonStarArity - prodArity
+      def prodArity    = extractor.prodArity
       def starArity    = patterns.starArity
       def totalArity   = patterns.totalArity
 
@@ -1693,15 +1693,15 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
       def typedNonStarPatterns = products ::: elements
       def typedPatterns        = typedNonStarPatterns ::: stars
 
-      def isBool   = !isSeq && productArity == 0
+      def isBool   = !isSeq && prodArity == 0
       def isSingle = !isSeq && totalArity == 1
       def isStar   = patterns.hasStar
       def isSeq    = extractor.hasSeq
 
       private def typedAsElement(pat: Pattern)  = TypedPat(pat, extractor.elementType)
       private def typedAsSequence(pat: Pattern) = TypedPat(pat, extractor.sequenceType)
-      private def productPats = patterns.fixed take productArity
-      private def elementPats = patterns.fixed drop productArity
+      private def productPats = patterns.fixed take prodArity
+      private def elementPats = patterns.fixed drop prodArity
       private def products    = (productPats, productTypes).zipped map TypedPat
       private def elements    = elementPats map typedAsElement
       private def stars       = patterns.starPatterns map typedAsSequence
@@ -1710,7 +1710,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
       |Aligned {
       |   patterns  $patterns
       |  extractor  $extractor
-      |    arities  $productArity/$elementArity/$starArity  // product/element/star
+      |    arities  $prodArity/$elementArity/$starArity  // product/element/star
       |      typed  ${typedPatterns mkString ", "}
       |}""".stripMargin.trim
     }
@@ -1826,7 +1826,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
         def offering      = extractor.offeringString
         def symString     = tree.symbol.showLocated
         def offerString   = if (extractor.isErroneous) "" else s" offering $offering"
-        def arityExpected = ( if (extractor.hasSeq) "at least " else "" ) + productArity
+        def arityExpected = ( if (extractor.hasSeq) "at least " else "" ) + prodArity
 
         def err(msg: String)         = ctx.error(msg, tree.pos)
         def warn(msg: String)        = ctx.warning(msg, tree.pos)
@@ -1871,12 +1871,12 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
           *  process, we will tuple the extractor before creation Aligned so that
           *  it contains known good values.
           */
-        def productArity    = extractor.productArity
+        def prodArity       = extractor.prodArity
         def acceptMessage   = if (extractor.isErroneous) "" else s" to hold ${extractor.offeringString}"
-        val requiresTupling = isUnapply && patterns.totalArity == 1 && productArity > 1
+        val requiresTupling = isUnapply && patterns.totalArity == 1 && prodArity > 1
 
         //if (requiresTupling && effectivePatternArity(args) == 1)
-        //  currentUnit.deprecationWarning(sel.pos, s"${sel.symbol.owner} expects $productArity patterns$acceptMessage but crushing into $productArity-tuple to fit single pattern (SI-6675)")
+        //  currentUnit.deprecationWarning(sel.pos, s"${sel.symbol.owner} expects $prodArity patterns$acceptMessage but crushing into $prodArity-tuple to fit single pattern (SI-6675)")
 
         val normalizedExtractor =
           if (requiresTupling)

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

@@ -29,6 +29,9 @@ class SymUtils(val self: Symbol) extends AnyVal {
 
   def isVolatile(implicit ctx: Context) = self.hasAnnotation(defn.VolatileAnnot)
 
+  def isAnyOverride(implicit ctx: Context) = self.is(Override) || self.is(AbsOverride)
+    // careful: AbsOverride is a term only flag. combining with Override would catch only terms.
+
   /** If this is a constructor, its owner: otherwise this. */
   final def skipConstructor(implicit ctx: Context): Symbol =
     if (self.isConstructor) self.owner else self