Simplify implementation of & on denotations

Author: odersky

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

@@ -385,7 +385,7 @@ 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 the intersection (using `&` or `safe_&` if `safeIntersection` is true)
+     *  is the intersection using `&` or `safe_&` if `safeIntersection` is true)
      *  of the infos of the operand denotations.
      */
     def & (that: Denotation, pre: Type, safeIntersection: Boolean = false)(implicit ctx: Context): Denotation = {
@@ -412,18 +412,21 @@ object Denotations {
         val sym1 = denot1.symbol
         val sym2 = denot2.symbol
 
-        val sym2Accessible = sym2.isAccessibleFrom(pre)
-
         /** Does `sym1` come before `sym2` in the linearization of `pre`? */
-        def precedes(sym1: Symbol, sym2: Symbol) = {
-          def precedesIn(bcs: List[ClassSymbol]): Boolean = bcs match
-            case bc :: bcs1 => (sym1 eq bc) || !(sym2 eq bc) && precedesIn(bcs1)
-            case Nil => false
-
-          (sym1 ne sym2) &&
-            (sym1.derivesFrom(sym2) ||
-              !sym2.derivesFrom(sym1) && precedesIn(pre.baseClasses))
-        }
+        def linearScore(owner1: Symbol, owner2: Symbol): Int =
+
+          def searchBaseClasses(bcs: List[ClassSymbol]): Int = bcs match
+            case bc :: bcs1 =>
+              if bc eq owner1 then 1
+              else if bc eq owner2 then -1
+              else searchBaseClasses(bcs1)
+            case Nil => 0
+
+          if owner1 eq owner2 then 0
+          else if owner1.derivesFrom(owner2) then 1
+          else if owner2.derivesFrom(owner1) then -1
+          else searchBaseClasses(pre.baseClasses)
+        end linearScore
 
         /** Similar to SymDenotation#accessBoundary, but without the special cases. */
         def accessBoundary(sym: Symbol) =
@@ -432,82 +435,49 @@ object Denotations {
             if (sym.is(Protected)) sym.owner.enclosingPackageClass
             else defn.RootClass)
 
-        /** Establish a partial order "preference" order between symbols.
-         *  Give preference to `sym1` over `sym2` if one of the following
-         *  conditions holds, in decreasing order of weight:
-         *   1. sym2 doesn't exist
-         *   2. sym1 is concrete and sym2 is abstract
-         *   3. The owner of sym1 comes before the owner of sym2 in the linearization
-         *      of the type of the prefix `pre`.
-         *   4. The access boundary of sym2 is properly contained in the access
-         *      boundary of sym1. For protected access, we count the enclosing
-         *      package as access boundary.
-         *   5. sym1 is a method but sym2 is not.
-         *  The aim of these criteria is to give some disambiguation on access which
-         *   - does not depend on textual order or other arbitrary choices
-         *   - minimizes raising of doubleDef errors
-         */
-        def preferSym(sym1: Symbol, sym2: Symbol) =
-          sym1.eq(sym2)
-          || sym1.exists
-             && (!sym2.exists
-                || sym1.isAsConcrete(sym2)
-                   && (!sym2.isAsConcrete(sym1)
-                      || precedes(sym1.owner, sym2.owner)
-                      || accessBoundary(sym2).isProperlyContainedIn(accessBoundary(sym1))
-                      || sym2.is(Bridge) && !sym1.is(Bridge)
-                      || sym1.is(Method) && !sym2.is(Method))
-                || sym1.info.isErroneous)
-
-        /** Sym preference provided types also override */
-        def prefer(sym1: Symbol, sym2: Symbol, info1: Type, info2: Type) =
-          preferSym(sym1, sym2) &&
-          info1.overrides(info2, sym1.matchNullaryLoosely || sym2.matchNullaryLoosely, checkClassInfo = false)
-
-        /** Handle conflict where we have either definitions coming from the same class
-         *  that have matching signatures as seen from the current prefix, or we have
-         *  definitions from different classes that have the same signature but
-         *  conflicting infos. In these cases, do a last minute disambiguation
-         *  by picking one alternative according to the ranking
-         *
-         *    1. Non-bridge methods
-         *    2. non-methods
-         *    3. bridges
-         *    4. NoSymbol
-         *
-         *  If that fails, return a MultiDenotation with both alternatives
-         *  and rely on overloading resolution to pick one of them.
-         */
-        def handleConflict: Denotation =
-
-          def preferMethod(sym1: Symbol, sym2: Symbol): Boolean =
-            sym1.exists &&
-              (!sym2.exists
-              || sym2.is(Bridge) && !sym1.is(Bridge)
-              || sym1.is(Method) && !sym2.is(Method)
-              || sym1.info.isErroneous)
-
-          if preferMethod(sym1, sym2) then denot1
-          else if preferMethod(sym2, sym1) then denot2
-          else
-            overload.println(i"overloaded with same signature: ${sym1.showLocated}: $info1 / ${sym2.showLocated}: $info2")
-            MultiDenotation(denot1, denot2)
-        end handleConflict
-
-        if (sym2Accessible && prefer(sym2, sym1, info2, info1)) denot2
+        def isHidden(sym: Symbol) = sym.exists && !sym.isAccessibleFrom(pre)
+        val hidden1 = isHidden(sym1)
+        val hidden2 = isHidden(sym2)
+        if hidden1 && !hidden2 then denot2
+        else if hidden2 && !hidden1 then denot1
         else
-          val sym1Accessible = sym1.isAccessibleFrom(pre)
-          if (sym1Accessible && prefer(sym1, sym2, info1, info2)) denot1
-          else if (sym1Accessible && sym2.exists && !sym2Accessible) denot1
-          else if (sym2Accessible && sym1.exists && !sym1Accessible) denot2
-          else if isDoubleDef(sym1, sym2) then handleConflict
+          val symScore: Int =
+            if !sym1.exists then -2
+            else if !sym2.exists then 2
+            else if !sym1.isAsConcrete(sym2) then -1
+            else if !sym2.isAsConcrete(sym1) then 1
+            else
+              val linScore = linearScore(sym1.owner, sym2.owner)
+              if linScore != 0 then linScore
+              else
+                val boundary1 = accessBoundary(sym1)
+                val boundary2 = accessBoundary(sym2)
+                if boundary1.isProperlyContainedIn(boundary2) then -1
+                else if boundary2.isProperlyContainedIn(boundary1) then 1
+                else if sym1.is(Bridge) && !sym2.is(Bridge) then -2
+                else if sym2.is(Bridge) && !sym1.is(Bridge) then 2
+                else if sym2.is(Method) && !sym1.is(Method) then -2
+                else if sym1.is(Method) && !sym2.is(Method) then 2
+                else 0
+
+          val matchLoosely = sym1.matchNullaryLoosely || sym2.matchNullaryLoosely
+
+          if info2.overrides(info1, matchLoosely, checkClassInfo = false)
+             && symScore <= 0
+          then denot2
+          else if info1.overrides(info2, matchLoosely, checkClassInfo = false)
+                  && symScore >= 0
+          then denot1
           else
-            val sym = if preferSym(sym2, sym1) then sym2 else sym1
             val jointInfo = infoMeet(info1, info2, safeIntersection)
             if jointInfo.exists then
+              val sym = if symScore > 0 then sym1 else sym2
               JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor, pre)
+            else if symScore == 2 then denot1
+            else if symScore == -2 then denot2
             else
-              handleConflict
+              overload.println(i"overloaded with same signature: ${sym1.showLocated}: $info1 / ${sym2.showLocated}: $info2, info = ${info1.getClass}, ${info2.getClass}, $jointInfo")
+              MultiDenotation(denot1, denot2)
       end mergeSingleDenot
 
       if (this eq that) this
@@ -533,7 +503,7 @@ object Denotations {
     final def containsSym(sym: Symbol): Boolean = hasUniqueSym && (symbol eq sym)
   }
 
-  // ------ Info meets and joins ---------------------------------------------
+  // ------ Info meets ----------------------------------------------------
 
   /** Merge parameter names of lambda types. If names in corresponding positions match, keep them,
     *  otherwise generate new synthetic names.
@@ -542,82 +512,55 @@ object Denotations {
     (for ((name1, name2, idx) <- tp1.paramNames.lazyZip(tp2.paramNames).lazyZip(tp1.paramNames.indices))
       yield if (name1 == name2) name1 else tp1.companion.syntheticParamName(idx)).toList
 
-  /** Normally, `tp1 & tp2`
-    *  Special cases for matching methods and classes, with
-    *  the possibility of returning NoType.
-    *  Special handling of ExprTypes, where mixed intersections widen the ExprType away.
-    */
+  /** Normally, `tp1 & tp2`, with extra care taken to return `tp1` or `tp2` directly if that's
+   *  a valid answer. Special cases for matching methods and classes, with
+   *  the possibility of returning NoType. Special handling of ExprTypes, where mixed
+   *  intersections widen the ExprType away.
+   */
   def infoMeet(tp1: Type, tp2: Type, safeIntersection: Boolean)(implicit ctx: Context): Type =
-    if (tp1 eq tp2) tp1
-    else tp1 match {
+    if tp1 eq tp2 then tp1
+    else tp1 match
       case tp1: TypeBounds =>
-        tp2 match {
-          case tp2: TypeBounds => if (safeIntersection) tp1 safe_& tp2 else tp1 & tp2
+        tp2 match
+          case tp2: TypeBounds => if safeIntersection then tp1 safe_& tp2 else tp1 & tp2
           case tp2: ClassInfo => tp2
           case _ => NoType
-        }
       case tp1: ClassInfo =>
-        tp2 match {
+        tp2 match
           case tp2: ClassInfo if tp1.cls eq tp2.cls => tp1.derivedClassInfo(tp1.prefix & tp2.prefix)
           case tp2: TypeBounds => tp1
           case _ => NoType
-        }
-
-      // Two remedial strategies:
-      //
-      //  1. Prefer method types over poly types. This is necessary to handle
-      //     overloaded definitions like the following
-      //
-      //        def ++ [B >: A](xs: C[B]): D[B]
-      //        def ++ (xs: C[A]): D[A]
-      //
-      //     (Code like this is found in the collection strawman)
-      //
-      // 2. In the case of two method types or two polytypes with matching
-      //    parameters and implicit status, merge corresponding parameter
-      //    and result types.
       case tp1: MethodType =>
-        tp2 match {
+        tp2 match
           case tp2: MethodType
-          if ctx.typeComparer.matchingMethodParams(tp1, tp2) && (tp1.companion eq tp2.companion) =>
+          if ctx.typeComparer.matchingMethodParams(tp1, tp2)
+             && (tp1.isImplicitMethod || tp1.isContextualMethod) == (tp2.isImplicitMethod || tp2.isContextualMethod)
+             && tp1.isErasedMethod == tp2.isErasedMethod =>
             val resType = infoMeet(tp1.resType, tp2.resType.subst(tp2, tp1), safeIntersection)
             if resType.exists then
               tp1.derivedLambdaType(mergeParamNames(tp1, tp2), tp1.paramInfos, resType)
-            else
-              NoType
-          case _ =>
-            NoType
-        }
+            else NoType
+          case _ => NoType
       case tp1: PolyType =>
-        tp2 match {
-          case tp2: PolyType if ctx.typeComparer.matchingPolyParams(tp1, tp2) =>
+        tp2 match
+          case tp2: PolyType if sameLength(tp1.paramNames, tp2.paramNames) =>
             val resType = infoMeet(tp1.resType, tp2.resType.subst(tp2, tp1), safeIntersection)
             if resType.exists then
               tp1.derivedLambdaType(
                 mergeParamNames(tp1, tp2),
                 tp1.paramInfos.zipWithConserve(tp2.paramInfos)( _ & _ ),
                 resType)
-            else
-              NoType
-          case _ =>
-            NoType
-        }
+            else NoType
+          case _ => NoType
       case ExprType(rtp1) =>
-        tp2 match {
+        tp2 match
           case ExprType(rtp2) => ExprType(rtp1 & rtp2)
           case _ => infoMeet(rtp1, tp2, safeIntersection)
-        }
       case _ =>
         tp2 match
-          case _: MethodType | _: PolyType =>
-            NoType
-          case _ =>
-            try tp1 & tp2.widenExpr
-            catch
-              case ex: Throwable =>
-                println(i"error for meet: $tp1 &&& $tp2, ${tp1.getClass}, ${tp2.getClass}")
-                throw ex
-    }
+          case _: MethodType | _: PolyType => NoType
+          case _ => tp1 & tp2.widenExpr
+  end infoMeet
 
   /** A non-overloaded denotation */
   abstract class SingleDenotation(symbol: Symbol, initInfo: Type) extends Denotation(symbol, initInfo) {
@@ -1193,7 +1136,7 @@ object Denotations {
       if sd1.exists then
         if sd2.exists then
           throw TypeError(
-            em"""Failure to disambiguate oberloaded reference with
+            em"""Failure to disambiguate overloaded reference with
                 |  ${denot1.symbol.showLocated}: ${denot1.info}  and
                 |  ${denot2.symbol.showLocated}: ${denot2.info}""")
         else sd1

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

@@ -922,8 +922,8 @@ object SymDenotations {
         else true
       }
 
-      if (pre eq NoPrefix) true
-      else if (isAbsent()) false
+      if pre eq NoPrefix then true
+      else if isAbsent() then false
       else {
         val boundary = accessBoundary(owner)
 
@@ -2100,7 +2100,7 @@ object SymDenotations {
       var names = Set[Name]()
       def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name
       try {
-        for (p <- classParents)
+        for (p <- classParents if p.classSymbol.isClass)
           for (name <- p.classSymbol.asClass.memberNames(keepOnly))
             maybeAdd(name)
         val ownSyms =

tests/neg-custom-args/allow-double-bindings/i1240.scala

@@ -8,7 +8,7 @@ class C[T] {
 
 object C {
   def main(args: Array[String]) =
-        new C[D]().foo(new D()) // error: ambiguous
+        new C[D]().foo(new D())
 }
 
 class C1[T] {

tests/neg/i4470a.scala

@@ -1,7 +1,7 @@
 object RepeatedEnum {
 
   enum Maybe { // error
-    case Foo   // error
+    case Foo
   }
 
   enum Maybe { // error

tests/neg/i4470b.scala

@@ -1,6 +1,6 @@
 object RepeatedExtendEnum {
 
-  enum Maybe[T] derives Eql { // error // error
+  enum Maybe[T] derives Eql { // error
     case Foo extends Maybe[Int]
   }
 

tests/neg/i4470c.scala

@@ -1,6 +1,6 @@
 object DuplicatedEnum {
   enum Maybe[+T] { // error
-    case Some(x: T) // error
+    case Some(x: T)
   }
 
   enum Maybe[+T] { // error