Apply suggestions from code review

dotty-staging · Jul 31, 2024 · 462b293 · 462b293
1 parent bba0db3
commit 462b293
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 21 deletions.
diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala
@@ -1991,11 +1991,10 @@ trait Applications extends Compatibility {
     // break that by changing implicit priority of types. On the other hand, we do
     // want to exhaust all other possibilities before using owner score as a tie breaker.
     // For instance, pos/scala-uri.scala depends on that.
-    def drawOrOwner =
-      if preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution) then
-        //println(i"disambi compare($alt1, $alt2)? $ownerScore")
-        ownerScore
-      else 0
+    def disambiguateWithOwner(result: Int) =
+      if result == 0 && preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution)
+      then ownerScore
+      else result
 
     if alt1.symbol.is(ConstructorProxy) && !alt2.symbol.is(ConstructorProxy) then -1
     else if alt2.symbol.is(ConstructorProxy) && !alt1.symbol.is(ConstructorProxy) then 1
@@ -2005,14 +2004,15 @@ trait Applications extends Compatibility {
       val strippedType1 = stripImplicit(fullType1)
       val strippedType2 = stripImplicit(fullType2)
 
-      val result = compareWithTypes(strippedType1, strippedType2)
-      if result != 0 then result
-      else if strippedType1 eq fullType1 then
+      var result = compareWithTypes(strippedType1, strippedType2)
+      if result != 0 then return result
+      if strippedType1 eq fullType1 then
         if strippedType2 eq fullType2
-        then drawOrOwner                          // no implicits either side: its' a draw
+        then disambiguateWithOwner(0)             // no implicits either side: its' a draw
         else 1                                    // prefer 1st alternative with no implicits
       else if strippedType2 eq fullType2 then -1  // prefer 2nd alternative with no implicits
-      else compareWithTypes(fullType1, fullType2) // continue by comparing implicits parameters
+      else disambiguateWithOwner(
+        compareWithTypes(fullType1, fullType2))   // continue by comparing implicit parameters
   }
   end compare
 

diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala
@@ -549,11 +549,10 @@ object Implicits:
   /** An ambiguous implicits failure */
   class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:
 
-    private[Implicits] var priorityChangeWarning: Message | Null = null
+    private[Implicits] var priorityChangeWarnings: List[Message] = Nil
 
     def priorityChangeWarningNote(using Context): String =
-      if priorityChangeWarning != null then s"\n\nNote: $priorityChangeWarning"
-      else ""
+      priorityChangeWarnings.map(msg => s"\n\nNote: $msg").mkString
 
     def msg(using Context): Message =
       var str1 = err.refStr(alt1.ref)
@@ -1627,15 +1626,23 @@ trait Implicits:
             throw ex
 
       val result = rank(sort(eligible), NoMatchingImplicitsFailure, Nil)
-      for (critical, msg) <- priorityChangeWarnings do
-        if result.found.exists(critical.contains(_)) then
-          result match
-            case result: SearchFailure =>
-              result.reason match
-                case ambi: AmbiguousImplicits => ambi.priorityChangeWarning = msg
-                case _ =>
+
+      // Issue all priority change warnings that can affect the result
+      val shownWarnings = priorityChangeWarnings.toList.collect:
+        case (critical, msg) if result.found.exists(critical.contains(_)) =>
+          msg
+      result match
+        case result: SearchFailure =>
+          result.reason match
+            case ambi: AmbiguousImplicits =>
+              // Make warnings part of error message because otherwise they are suppressed when
+              // the error is emitted.
+              ambi.priorityChangeWarnings = shownWarnings
             case _ =>
-          report.warning(msg, srcPos)
+        case _ =>
+      for msg <- shownWarnings do
+        report.warning(msg, srcPos)
+
       result
     end searchImplicit